Skip to content

Commit

Permalink
Fix busy issues with the history window (#4889)
Browse files Browse the repository at this point in the history
For #4853

Also address some linting errors from previous submissions.

<!--
  If an item below does not apply to you, then go ahead and check it off as "done" and strikethrough the text, e.g.:
    - [x] ~Has unit tests & system/integration tests~
-->
- [x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
- [x] Title summarizes what is changing
- [x] Has a [news entry](https://github.com/Microsoft/vscode-python/tree/master/news) file (remember to thank yourself!)
- [ ] Has sufficient logging.
- [ ] Has telemetry for enhancements.
- [x] Unit tests & system/integration tests are added/updated
- [ ] [Test plan](https://github.com/Microsoft/vscode-python/blob/master/.github/test_plan.md) is updated as appropriate
- [ ] [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package-lock.json) has been regenerated by running `npm install` (if dependencies have changed)
- [ ] The wiki is updated with any design decisions/details.
  • Loading branch information
rchiodo authored Mar 22, 2019
1 parent 2f55a0d commit 26a7b9c
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 21 deletions.
1 change: 1 addition & 0 deletions news/2 Fixes/4853.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix status bar when using Live Share or just starting the Python Interactive window.
1 change: 1 addition & 0 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
"DataScience.liveShareSyncFailure": "Synchronization failure during live share startup.",
"DataScience.liveShareServiceFailure": "Failure starting '{0}' service during live share connection.",
"DataScience.documentMismatch": "Cannot run cells, duplicate documents for {0} found.",
"DataScience.pythonInteractiveCreateFailed": "Failure to create a 'Python Interactive' window. Try reinstalling the Python extension.",
"diagnostics.warnSourceMaps": "Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.",
"diagnostics.disableSourceMaps": "Disable Source Map Support",
"diagnostics.warnBeforeEnablingSourceMaps": "Enabling source map support in the Python Extension will adversely impact performance of the extension.",
Expand Down
2 changes: 2 additions & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ export namespace DataScience {
export const liveShareServiceFailure = localize('DataScience.liveShareServiceFailure', 'Failure starting \'{0}\' service during live share connection.');
export const documentMismatch = localize('DataScience.documentMismatch', 'Cannot run cells, duplicate documents for {0} found.');
export const jupyterGetVariablesBadResults = localize('DataScience.jupyterGetVariablesBadResults', 'Failed to fetch variable info from the Jupyter server.');
export const pythonInteractiveCreateFailed = localize('DataScience.pythonInteractiveCreateFailed', 'Failure to create a \'Python Interactive\' window. Try reinstalling the Python extension.');

}

export namespace DebugConfigurationPrompts {
Expand Down
26 changes: 16 additions & 10 deletions src/client/datascience/history/historyProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as vsls from 'vsls/vscode';
import { ILiveShareApi, IWorkspaceService } from '../../common/application/types';
import { IAsyncDisposable, IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types';
import { createDeferred, Deferred } from '../../common/utils/async';
import * as localize from '../../common/utils/localize';
import { IServiceContainer } from '../../ioc/types';
import { Identifiers, LiveShare, LiveShareCommands, Settings } from '../constants';
import { PostOffice } from '../liveshare/postOffice';
Expand Down Expand Up @@ -64,14 +65,18 @@ export class HistoryProvider implements IHistoryProvider, IAsyncDisposable {

public async getOrCreateActive() : Promise<IHistory> {
if (!this.activeHistory) {
this.activeHistory = await this.create();
await this.create();
}

// Make sure all other providers have an active history.
await this.synchronizeCreate();

// Now that all of our peers have sync'd, return the history to use.
return this.activeHistory;
if (this.activeHistory) {
return this.activeHistory;
}

throw new Error(localize.DataScience.pythonInteractiveCreateFailed());
}

public async getNotebookOptions() : Promise<INotebookServerOptions> {
Expand Down Expand Up @@ -107,15 +112,16 @@ export class HistoryProvider implements IHistoryProvider, IAsyncDisposable {
return this.postOffice.dispose();
}

private async create() : Promise<IHistory> {
const result = this.serviceContainer.get<IHistory>(IHistory);
const handler = result.closed(this.onHistoryClosed);
this.disposables.push(result);
private async create() : Promise<void> {
// Set it as soon as we create it. The .ctor for the history window
// may cause a subclass to talk to the IHistoryProvider to get the active history.
this.activeHistory = this.serviceContainer.get<IHistory>(IHistory);
const handler = this.activeHistory.closed(this.onHistoryClosed);
this.disposables.push(this.activeHistory);
this.disposables.push(handler);
this.activeHistoryExecuteHandler = result.onExecutedCode(this.onHistoryExecute);
this.activeHistoryExecuteHandler = this.activeHistory.onExecutedCode(this.onHistoryExecute);
this.disposables.push(this.activeHistoryExecuteHandler);
await result.ready;
return result;
await this.activeHistory.ready;
}

private onPeerCountChanged(newCount: number) {
Expand All @@ -133,7 +139,7 @@ export class HistoryProvider implements IHistoryProvider, IAsyncDisposable {
// The other side is creating a history window. Create on this side. We don't need to show
// it as the running of new code should do that.
if (!this.activeHistory) {
this.activeHistory = await this.create();
await this.create();
}

// Tell the requestor that we got its message (it should be waiting for all peers to sync)
Expand Down
14 changes: 8 additions & 6 deletions src/client/telemetry/importTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ export class ImportTracker implements IImportTracker {
this.documentManager.textDocuments.forEach(d => this.onOpenedOrSavedDocument(d));
}

private getDocumentLines(document: TextDocument) : string [] {
return Array.apply(null, {length: Math.min(document.lineCount, MAX_DOCUMENT_LINES)}).map((a, i) => {
private getDocumentLines(document: TextDocument) : (string | undefined)[] {
const array = new Array(null, Math.min(document.lineCount, MAX_DOCUMENT_LINES));
return array.map((_a: any, i: number) => {
const line = document.lineAt(i);
if (line && !line.isEmptyOrWhitespace) {
return line.text;
Expand All @@ -74,8 +75,9 @@ export class ImportTracker implements IImportTracker {

private scheduleDocument(document: TextDocument) {
// If already scheduled, cancel.
if (this.pendingDocs.has(document.fileName)) {
clearTimeout(this.pendingDocs.get(document.fileName));
const currentTimeout = this.pendingDocs.get(document.fileName);
if (currentTimeout) {
clearTimeout(currentTimeout);
this.pendingDocs.delete(document.fileName);
}

Expand All @@ -100,12 +102,12 @@ export class ImportTracker implements IImportTracker {
this.lookForImports(lines, EventName.KNOWN_IMPORT_FROM_EXECUTION);
}

private lookForImports(lines: string[], eventName: string) {
private lookForImports(lines: (string | undefined)[], eventName: string) {
try {
// Use a regex to parse each line, looking for imports
const matches: Set<string> = new Set<string>();
for (const s of lines) {
const match = ImportRegEx.exec(s);
const match = s ? ImportRegEx.exec(s) : null;
if (match && match.length > 2) {
// Could be a from or a straight import. from is the first entry.
const actual = match[1] ? match[1] : match[2];
Expand Down
9 changes: 4 additions & 5 deletions src/test/datascience/liveshare.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,14 @@ suite('LiveShare tests', () => {
// The history provider create needs to be rewritten to make the history window think the mounted web panel is
// ready.
const origFunc = (historyProvider as any).create.bind(historyProvider);
(historyProvider as any).create = async (): Promise<IHistory> => {
const createResult = await origFunc();
(historyProvider as any).create = async (): Promise<void> => {
await origFunc();
const history = historyProvider.getActive();

// During testing the MainPanel sends the init message before our history is created.
// Pretend like it's happening now
const listener = ((createResult as any).messageListener) as HistoryMessageListener;
const listener = ((history as any).messageListener) as HistoryMessageListener;
listener.onMessage(HistoryMessages.Started, {});

return createResult;
};

return result;
Expand Down

0 comments on commit 26a7b9c

Please sign in to comment.