Add support for latest version of PTVSD as an experimental debugger by DonJayamanne · Pull Request #745 · microsoft/vscode-python

DonJayamanne · 2018-02-09T19:53:06Z

Fixes #740

Archive of 0.7.0

* 'master' of https://github.com/Microsoft/vscode-python: Fixes #56 list all environments (#219) Fixes #57 Disable activation on debugging (#220) Fixes #26 Do not run linters when linters are disabled (#222)

* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)

* upstream/master: Fix feedback service (#246) Fix django context initializer (#248) disable generation of tags file upon extension load (#264)

* upstream/master: Resolve pythonPath before comparing it to shebang (#273)

* upstream/master: Fixes #22 to Detect anaconda from known locations (#221) Use workspaceFolder token instead of workspaceRoot (#267) Fix registry lookup response (#224) Fix issues when running without debugging and debugged code terminates (#249)

* upstream/master: Fix debugging tests (#304)

* upstream/master: Remove jupyter functionality in favor of Jupyter extension (#302) Drop Python 2 URLs (#307)

* upstream/master: Remove setting python.formatting.formatOnSave in favor of the vs code setting (#312)

* upstream/master: Remove setting linting.lintOnTextChange as it was never implemented (#315)

* upstream/master: Fix travis build error (#326)

* upstream/master: add new npm deps with improved gulp for dev (#328)

* upstream/master: Update version of inversify package (#329)

* upstream/master: Document our dev process (#330)

* upstream/master: Document contribution to the code along with coding standards (#321)

* upstream/master: Add Simplified Chinese translation of commands (#240)

* upstream/master: Fix package.json (#347)

* upstream/master: #34, #110 - suppress Intellisense in strings and comments (#339) Re-factor code python execution framework (#345)

* upstream/master: Fix linters to make use of the new python code execution framework (#360) Update the versioning scheme (#356) Make npm happy in regards to line endings (#357)

* upstream/master: Ensure python path is not set if already set in user settings (#369) Use 'an' rather than 'a' before vowel words (#373)

* upstream/master: Use new environment variable parser (#362)

MikhailArkhipov · 2018-02-09T22:42:42Z

src/client/debugger/Common/messages.ts

+import { DebugProtocol } from 'vscode-debugprotocol';
+
+export class Event extends Message implements DebugProtocol.Event {
+    public event: string;


Removed it, found that VS Code has already got this.

One less file.

MikhailArkhipov · 2018-02-09T22:44:03Z

src/client/debugger/Common/protocolLogger.ts

+        this.messagesToLog = [];
+    }
+    private fromDataCallbackHandler = (data: string | Buffer) => {
+        this.logMessages(['From Client:', (data as Buffer).toString('utf8')]);


Looks like it casts data to buffer even if it is string?

Yes, we're only dealing with buffer here, this is a logger for the protocol stream, we'll never handle strings. never.

The problem is streams have a generic call back of either string or buffer,
Either way this won't be a problem as we're just using the 'toString' method.
I need to cast it so that I can pass the arg.

MikhailArkhipov · 2018-02-09T22:45:22Z

src/client/debugger/Common/protocolParser.ts

+    private stream?: Readable;
+    constructor() {
+        super();
+        this.contentLength = -1;


just set it right at the declaration?

MikhailArkhipov · 2018-02-09T22:56:27Z

src/client/debugger/DebugClients/LocalDebugClient.ts

-        this.pythonProcess = pythonProcess;
-        this.debugServer = new LocalDebugServer(this.debugSession, this.pythonProcess, this.args);
+    public CreateDebugServer(pythonProcess?: IPythonProcess, serviceContainer?: IServiceContainer): BaseDebugServer {
+        if ((this.args as LaunchRequestArguments).type === 'pythonExperimental') {


ctor receives args as LaunchRequestArguments - do we still need to cast?

Yes, this is a generic class used for remote and local debugging. In remote debugging the args is a different type.
The property args is any in the base class. Could probably use generics, but didn't want to make too many changes to the existing debugger.

Ok, thats a lame excuse, will fix it. lol

* upstream/master: Limit Jedi RAM consumption (#744) Fix debug test failure (#739) Ensure paths are always using forward slashes in terminal commands and args (#732)

* 'master' of https://github.com/DonJayamanne/pythonVSCode:

int19h

My TS-fu is weak, so this review is mostly about protocol handling.

DonJayamanne · 2018-02-10T01:32:16Z

My TS-fu is weak, so this review is mostly about protocol handling.

Yes it is.

MikhailArkhipov · 2018-02-10T04:17:59Z

src/client/debugger/DebugClients/RemoteDebugClient.ts

-        this.pythonProcess = pythonProcess;
-        this.debugServer = new RemoteDebugServer(this.debugSession, this.pythonProcess, this.args);
+    public CreateDebugServer(pythonProcess?: IPythonProcess): BaseDebugServer {
+        this.pythonProcess = pythonProcess!;


So it is never actually null or can it be? If not, then ? prob not needed

Optional parameter for other implementations of this class.

MikhailArkhipov · 2018-02-10T04:19:53Z

src/client/debugger/Main.ts

-        this.sendEvent(new TerminatedEvent());
-    }
    protected attachRequest(response: DebugProtocol.AttachResponse, args: AttachRequestArguments) {
+        if ((args as any).diagnosticLogging === true) {


do we need to cast to any if is strongly typed?

MikhailArkhipov · 2018-02-10T04:20:33Z

src/client/debugger/mainV2.ts

+import { IDebugStreamProvider, IProtocolLogger, IProtocolMessageWriter, IProtocolParser } from './types';
+
+export class PythonDebugger extends DebugSession {
+    public debugServer?: BaseDebugServer;


readonly for public fields? Or turn into getters? At least it is preferable practice in C#

MikhailArkhipov · 2018-02-10T04:22:46Z

src/client/debugger/mainV2.ts

+
+            // Lets start our debugger.
+            const session = new PythonDebugger(serviceContainer, isServerMode);
+            session.setRunAsServer(isServerMode);


if we are passing isServerMode down to the session ctor, can it setRunAsServer internally?

MikhailArkhipov · 2018-02-10T04:23:01Z

src/client/debugger/mainV2.ts

+
+            function dispose() {
+                if (session) {
+                    session.shutdown();


session = undefined?

MikhailArkhipov · 2018-02-10T04:26:17Z

src/client/debugger/Main.ts

    }
    @capturePerformanceTelemetry('launch')
    protected launchRequest(response: DebugProtocol.LaunchResponse, args: LaunchRequestArguments): void {
+        if ((args as any).diagnosticLogging === true) {


Do we need to cast to any?

octref and others added 30 commits November 3, 2017 13:11

Fix microsoft/vscode#37627 (#1368)

ecc1ca9

Version 0.7.0 of extension (#1381)

7c5778c

Archive of 0.7.0

Update README.md

9d1bf82

Update README.md

ffba179

sync fork with upstream

905c713

fix readme

acc2109

merged upstream

d392e8b

Merge remote-tracking branch 'upstream/master'

92f775f

* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)

Merge remote-tracking branch 'upstream/master'

32a6e53

* upstream/master: Fix feedback service (#246) Fix django context initializer (#248) disable generation of tags file upon extension load (#264)

Merge remote-tracking branch 'upstream/master'

4b30f2c

* upstream/master: Resolve pythonPath before comparing it to shebang (#273)

Merge remote-tracking branch 'upstream/master'

eff4792

Merge remote-tracking branch 'upstream/master'

4553c28

Merge remote-tracking branch 'upstream/master'

3c6520a

* upstream/master: Fix debugging tests (#304)

Merge remote-tracking branch 'upstream/master'

966e516

* upstream/master: Remove jupyter functionality in favor of Jupyter extension (#302) Drop Python 2 URLs (#307)

Merge remote-tracking branch 'upstream/master'

63d2d65

* upstream/master: Remove setting python.formatting.formatOnSave in favor of the vs code setting (#312)

Merge remote-tracking branch 'upstream/master'

f6d469e

* upstream/master: Remove setting linting.lintOnTextChange as it was never implemented (#315)

Merge remote-tracking branch 'upstream/master'

029e055

* upstream/master: Fix travis build error (#326)

Merge remote-tracking branch 'upstream/master'

e8c71c0

* upstream/master: add new npm deps with improved gulp for dev (#328)

Merge remote-tracking branch 'upstream/master'

51cf9d2

* upstream/master: Update version of inversify package (#329)

Merge remote-tracking branch 'upstream/master'

7aadc43

* upstream/master: Document our dev process (#330)

Merge remote-tracking branch 'upstream/master'

f0f5c59

* upstream/master: Document contribution to the code along with coding standards (#321)

Merge remote-tracking branch 'upstream/master'

b2b9da9

* upstream/master: Add Simplified Chinese translation of commands (#240)

Merge remote-tracking branch 'upstream/master'

30a4091

* upstream/master: Fix package.json (#347)

Merge remote-tracking branch 'upstream/master'

b16d2f9

* upstream/master: #34, #110 - suppress Intellisense in strings and comments (#339) Re-factor code python execution framework (#345)

Merge remote-tracking branch 'upstream/master'

c8db345

* upstream/master: Fix linters to make use of the new python code execution framework (#360) Update the versioning scheme (#356) Make npm happy in regards to line endings (#357)

Merge remote-tracking branch 'upstream/master'

0df7f16

* upstream/master: Ensure python path is not set if already set in user settings (#369) Use 'an' rather than 'a' before vowel words (#373)

Merge remote-tracking branch 'upstream/master'

3ccc881

* upstream/master: Use new environment variable parser (#362)

Merge remote-tracking branch 'upstream/master'

bb0709e

MikhailArkhipov reviewed Feb 9, 2018

View reviewed changes

🔨 ✨ enabled debugging in terminals with minor changes

d64db82

MikhailArkhipov reviewed Feb 9, 2018

View reviewed changes

DonJayamanne added 12 commits February 9, 2018 15:07

👌 code review fixes

85c16b1

👌 use generics to clean code

6ed650e

👌 code review fixes

b941887

👌 code review fixes

8f49e96

Merge remote-tracking branch 'upstream/master'

aeade06

* upstream/master: Limit Jedi RAM consumption (#744) Fix debug test failure (#739) Ensure paths are always using forward slashes in terminal commands and args (#732)

Merge branch 'master' of https://github.com/DonJayamanne/pythonVSCode

6850da2

* 'master' of https://github.com/DonJayamanne/pythonVSCode:

💄 indentation changes to package.json

5d190a4

merged master

289c9e7

🐛 fix merge issue

15fe650

📦 update yarn lock

9935ac0

📝 changes to labels

d47eef0

🔨 updated to ensure capabilities line up with ptvsd

14b2dd2

int19h approved these changes Feb 10, 2018

View reviewed changes

MikhailArkhipov reviewed Feb 10, 2018

View reviewed changes

MikhailArkhipov approved these changes Feb 10, 2018

View reviewed changes

DonJayamanne added 3 commits February 11, 2018 13:04

🔨 ensure debug console is not an option

c16724a

👌 strongly typed logging settings

713d582

🔨 limit debug options

de145fc

DonJayamanne merged commit 937470e into microsoft:master Feb 11, 2018

DonJayamanne deleted the experimentalDebugger branch February 22, 2018 19:22

lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019

Conversation

DonJayamanne commented Feb 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

int19h left a comment

Choose a reason for hiding this comment

Uh oh!

DonJayamanne commented Feb 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikhailArkhipov Feb 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MikhailArkhipov Feb 10, 2018 •

edited

Loading