Skip to content

Commit 1575c12

Browse files
authored
Fix issues with LiveShare and triple quotes (#5084)
For #5012 and bugs found while testing the fix for 5012 around liveshare <!-- 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!) - [x] 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 c95e97f commit 1575c12

15 files changed

Lines changed: 198 additions & 113 deletions

build/ci/vscode-python-ci.yaml

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ variables:
1818
NpmVersion: 'latest'
1919
MOCHA_FILE: '$(Build.ArtifactStagingDirectory)/test-junit.xml' # All test files will write their JUnit xml output to this file, clobbering the last time it was written.
2020
MOCHA_REPORTER_JUNIT: true # Use the mocha-multi-reporters and send output to both console (spec) and JUnit (mocha-junit-reporter).
21-
# VSC_PYTHON_FORCE_LOGGING: true - Enable this to turn on console output for the logger
21+
VSC_PYTHON_FORCE_LOGGING: true # Enable this to turn on console output for the logger
2222

2323
jobs:
2424

@@ -213,47 +213,41 @@ jobs:
213213
NeedsPythonFunctionalReqs: true
214214
# This tells the functional tests to not mock out Jupyter...
215215
VSCODE_PYTHON_ROLLING: true
216-
VSC_PYTHON_FORCE_LOGGING: true
217216
'Linux-Py3.7 Functional':
218217
PythonVersion: '3.7'
219218
VMImageName: 'ubuntu-16.04'
220219
TestsToRun: 'testfunctional'
221220
NeedsPythonTestReqs: true
222221
NeedsPythonFunctionalReqs: true
223222
VSCODE_PYTHON_ROLLING: true
224-
VSC_PYTHON_FORCE_LOGGING: true
225223
'Mac-Py3.7 Functional':
226224
PythonVersion: '3.7'
227225
VMImageName: 'macos-10.13'
228226
TestsToRun: 'testfunctional'
229227
NeedsPythonTestReqs: true
230228
NeedsPythonFunctionalReqs: true
231229
VSCODE_PYTHON_ROLLING: true
232-
VSC_PYTHON_FORCE_LOGGING: true
233230
'Windows-Py3.6 Functional':
234231
PythonVersion: '3.6'
235232
VMImageName: 'vs2017-win2016'
236233
TestsToRun: 'testfunctional'
237234
NeedsPythonTestReqs: true
238235
NeedsPythonFunctionalReqs: true
239236
VSCODE_PYTHON_ROLLING: true
240-
VSC_PYTHON_FORCE_LOGGING: true
241237
'Linux-Py3.6 Functional':
242238
PythonVersion: '3.6'
243239
VMImageName: 'ubuntu-16.04'
244240
TestsToRun: 'testfunctional'
245241
NeedsPythonTestReqs: true
246242
NeedsPythonFunctionalReqs: true
247243
VSCODE_PYTHON_ROLLING: true
248-
VSC_PYTHON_FORCE_LOGGING: true
249244
'Mac-Py3.6 Functional':
250245
PythonVersion: '3.6'
251246
VMImageName: 'macos-10.13'
252247
TestsToRun: 'testfunctional'
253248
NeedsPythonTestReqs: true
254249
NeedsPythonFunctionalReqs: true
255250
VSCODE_PYTHON_ROLLING: true
256-
VSC_PYTHON_FORCE_LOGGING: true
257251

258252
pool:
259253
vmImage: $(VMImageName)

news/2 Fixes/5012.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix triple quoted comments in cells to not affect anything

src/client/datascience/history/history.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
import { CancellationError } from '../../common/cancellation';
2424
import { EXTENSION_ROOT_DIR } from '../../common/constants';
2525
import { ContextKey } from '../../common/contextKey';
26+
import { traceInfo } from '../../common/logger';
2627
import { IFileSystem } from '../../common/platform/types';
2728
import { IConfigurationService, IDisposable, IDisposableRegistry, ILogger } from '../../common/types';
2829
import { createDeferred, Deferred } from '../../common/utils/async';
@@ -594,6 +595,7 @@ export class History implements IHistory {
594595

595596
// Wait for the cell to finish
596597
await finishedAddingCode.promise;
598+
traceInfo(`Finished execution for ${id}`);
597599
}
598600
} catch (err) {
599601
status.dispose();

src/client/datascience/jupyter/jupyterServer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { createDeferred, Deferred, sleep } from '../../common/utils/async';
2121
import * as localize from '../../common/utils/localize';
2222
import { noop } from '../../common/utils/misc';
2323
import { generateCells } from '../cellFactory';
24-
import { concatMultilineString, stripComments } from '../common';
24+
import { concatMultilineString } from '../common';
2525
import { Identifiers } from '../constants';
2626
import {
2727
CellState,
@@ -563,7 +563,7 @@ export class JupyterServerBase implements INotebookServer {
563563
subscriber.error(this.sessionStartTime, new Error(localize.DataScience.jupyterServerCrashed().format(exitCode.toString())));
564564
subscriber.complete(this.sessionStartTime);
565565
} else {
566-
const request = this.generateRequest(concatMultilineString(stripComments(subscriber.cell.data.source)), silent);
566+
const request = this.generateRequest(concatMultilineString(subscriber.cell.data.source), silent);
567567

568568
// tslint:disable-next-line:no-require-imports
569569
const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');

src/client/datascience/jupyter/jupyterServerFactory.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,12 @@ export class JupyterServerFactory implements INotebookServer {
103103
return new Observable<ICell[]>(subscriber => {
104104
this.serverFactory.get().then(s => {
105105
s.executeObservable(code, file, line, id, silent)
106-
.forEach(n => subscriber.next(n), Promise)
107-
.then(_f => subscriber.complete())
106+
.forEach(n => {
107+
subscriber.next(n); // Separate lines so can break on this call.
108+
}, Promise)
109+
.then(_f => {
110+
subscriber.complete();
111+
})
108112
.catch(e => subscriber.error(e));
109113
},
110114
r => {

src/client/datascience/jupyter/jupyterSession.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ export class JupyterSession implements IJupyterSession {
171171
}
172172

173173
private onStatusChanged(_s: Session.ISession, a: Kernel.Status) {
174-
traceInfo(`Status changed to ${a} from OnStatusChangedEvent`);
175174
if (a === 'starting' && this.onRestartedEvent) {
176175
this.onRestartedEvent.fire();
177176
}

src/client/datascience/jupyter/liveshare/guestJupyterServer.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as vsls from 'vsls/vscode';
77

88
import { ILiveShareApi } from '../../../common/application/types';
99
import { CancellationError } from '../../../common/cancellation';
10+
import { traceInfo } from '../../../common/logger';
1011
import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, ILogger } from '../../../common/types';
1112
import { createDeferred, Deferred } from '../../../common/utils/async';
1213
import * as localize from '../../../common/utils/localize';
@@ -22,7 +23,7 @@ import {
2223
} from '../../types';
2324
import { LiveShareParticipantDefault, LiveShareParticipantGuest } from './liveShareParticipantMixin';
2425
import { ResponseQueue } from './responseQueue';
25-
import { ILiveShareParticipant, IServerResponse } from './types';
26+
import { IExecuteObservableResponse, ILiveShareParticipant, IServerResponse } from './types';
2627

2728
export class GuestJupyterServer
2829
extends LiveShareParticipantGuest(LiveShareParticipantDefault, LiveShare.JupyterServerSharedService)
@@ -104,7 +105,7 @@ export class GuestJupyterServer
104105
s.notify(LiveShareCommands.executeObservable, { code, file, line, id });
105106
}
106107
}).ignoreErrors();
107-
return this.responseQueue.waitForObservable(code, file, line, id);
108+
return this.responseQueue.waitForObservable(code, id);
108109
}
109110

110111
public async restartKernel(): Promise<void> {
@@ -176,6 +177,8 @@ export class GuestJupyterServer
176177
}
177178

178179
private onServerResponse = (args: Object) => {
180+
const er = args as IExecuteObservableResponse;
181+
traceInfo(`Guest serverResponse ${er.pos} ${er.id}`);
179182
// Args should be of type ServerResponse. Stick in our queue if so.
180183
if (args.hasOwnProperty('type')) {
181184
this.responseQueue.push(args as IServerResponse);

0 commit comments

Comments
 (0)