Skip to content

Commit 26a7b9c

Browse files
rchiodorchiodo
authored
Fix busy issues with the history window (microsoft#4889)
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.
1 parent 2f55a0d commit 26a7b9c

6 files changed

Lines changed: 32 additions & 21 deletions

File tree

‎news/2 Fixes/4853.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix status bar when using Live Share or just starting the Python Interactive window.

‎package.nls.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@
158158
"DataScience.liveShareSyncFailure": "Synchronization failure during live share startup.",
159159
"DataScience.liveShareServiceFailure": "Failure starting '{0}' service during live share connection.",
160160
"DataScience.documentMismatch": "Cannot run cells, duplicate documents for {0} found.",
161+
"DataScience.pythonInteractiveCreateFailed": "Failure to create a 'Python Interactive' window. Try reinstalling the Python extension.",
161162
"diagnostics.warnSourceMaps": "Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.",
162163
"diagnostics.disableSourceMaps": "Disable Source Map Support",
163164
"diagnostics.warnBeforeEnablingSourceMaps": "Enabling source map support in the Python Extension will adversely impact performance of the extension.",

‎src/client/common/utils/localize.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ export namespace DataScience {
143143
export const liveShareServiceFailure = localize('DataScience.liveShareServiceFailure', 'Failure starting \'{0}\' service during live share connection.');
144144
export const documentMismatch = localize('DataScience.documentMismatch', 'Cannot run cells, duplicate documents for {0} found.');
145145
export const jupyterGetVariablesBadResults = localize('DataScience.jupyterGetVariablesBadResults', 'Failed to fetch variable info from the Jupyter server.');
146+
export const pythonInteractiveCreateFailed = localize('DataScience.pythonInteractiveCreateFailed', 'Failure to create a \'Python Interactive\' window. Try reinstalling the Python extension.');
147+
146148
}
147149

148150
export namespace DebugConfigurationPrompts {

‎src/client/datascience/history/historyProvider.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as vsls from 'vsls/vscode';
99
import { ILiveShareApi, IWorkspaceService } from '../../common/application/types';
1010
import { IAsyncDisposable, IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types';
1111
import { createDeferred, Deferred } from '../../common/utils/async';
12+
import * as localize from '../../common/utils/localize';
1213
import { IServiceContainer } from '../../ioc/types';
1314
import { Identifiers, LiveShare, LiveShareCommands, Settings } from '../constants';
1415
import { PostOffice } from '../liveshare/postOffice';
@@ -64,14 +65,18 @@ export class HistoryProvider implements IHistoryProvider, IAsyncDisposable {
6465

6566
public async getOrCreateActive() : Promise<IHistory> {
6667
if (!this.activeHistory) {
67-
this.activeHistory = await this.create();
68+
await this.create();
6869
}
6970

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

7374
// Now that all of our peers have sync'd, return the history to use.
74-
return this.activeHistory;
75+
if (this.activeHistory) {
76+
return this.activeHistory;
77+
}
78+
79+
throw new Error(localize.DataScience.pythonInteractiveCreateFailed());
7580
}
7681

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

110-
private async create() : Promise<IHistory> {
111-
const result = this.serviceContainer.get<IHistory>(IHistory);
112-
const handler = result.closed(this.onHistoryClosed);
113-
this.disposables.push(result);
115+
private async create() : Promise<void> {
116+
// Set it as soon as we create it. The .ctor for the history window
117+
// may cause a subclass to talk to the IHistoryProvider to get the active history.
118+
this.activeHistory = this.serviceContainer.get<IHistory>(IHistory);
119+
const handler = this.activeHistory.closed(this.onHistoryClosed);
120+
this.disposables.push(this.activeHistory);
114121
this.disposables.push(handler);
115-
this.activeHistoryExecuteHandler = result.onExecutedCode(this.onHistoryExecute);
122+
this.activeHistoryExecuteHandler = this.activeHistory.onExecutedCode(this.onHistoryExecute);
116123
this.disposables.push(this.activeHistoryExecuteHandler);
117-
await result.ready;
118-
return result;
124+
await this.activeHistory.ready;
119125
}
120126

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

139145
// Tell the requestor that we got its message (it should be waiting for all peers to sync)

‎src/client/telemetry/importTracker.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ export class ImportTracker implements IImportTracker {
5454
this.documentManager.textDocuments.forEach(d => this.onOpenedOrSavedDocument(d));
5555
}
5656

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

7576
private scheduleDocument(document: TextDocument) {
7677
// If already scheduled, cancel.
77-
if (this.pendingDocs.has(document.fileName)) {
78-
clearTimeout(this.pendingDocs.get(document.fileName));
78+
const currentTimeout = this.pendingDocs.get(document.fileName);
79+
if (currentTimeout) {
80+
clearTimeout(currentTimeout);
7981
this.pendingDocs.delete(document.fileName);
8082
}
8183

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

103-
private lookForImports(lines: string[], eventName: string) {
105+
private lookForImports(lines: (string | undefined)[], eventName: string) {
104106
try {
105107
// Use a regex to parse each line, looking for imports
106108
const matches: Set<string> = new Set<string>();
107109
for (const s of lines) {
108-
const match = ImportRegEx.exec(s);
110+
const match = s ? ImportRegEx.exec(s) : null;
109111
if (match && match.length > 2) {
110112
// Could be a from or a straight import. from is the first entry.
111113
const actual = match[1] ? match[1] : match[2];

‎src/test/datascience/liveshare.functional.test.tsx‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,14 @@ suite('LiveShare tests', () => {
145145
// The history provider create needs to be rewritten to make the history window think the mounted web panel is
146146
// ready.
147147
const origFunc = (historyProvider as any).create.bind(historyProvider);
148-
(historyProvider as any).create = async (): Promise<IHistory> => {
149-
const createResult = await origFunc();
148+
(historyProvider as any).create = async (): Promise<void> => {
149+
await origFunc();
150+
const history = historyProvider.getActive();
150151

151152
// During testing the MainPanel sends the init message before our history is created.
152153
// Pretend like it's happening now
153-
const listener = ((createResult as any).messageListener) as HistoryMessageListener;
154+
const listener = ((history as any).messageListener) as HistoryMessageListener;
154155
listener.onMessage(HistoryMessages.Started, {});
155-
156-
return createResult;
157156
};
158157

159158
return result;

0 commit comments

Comments
 (0)