FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix issues when running without debugging and debugged code terminates by DonJayamanne · Pull Request #249 · microsoft/vscode-python · GitHub

Fix issues when running without debugging and debugged code terminates - #249

Merged
Don Jayamanne (DonJayamanne) merged 27 commits into
microsoft:masterfrom
DonJayamanne:RunWithoutDebug
Nov 22, 2017
Merged

Fix issues when running without debugging and debugged code terminates#249
Don Jayamanne (DonJayamanne) merged 27 commits into
microsoft:masterfrom
DonJayamanne:RunWithoutDebug

Conversation

Don Jayamanne (DonJayamanne) commented Nov 16, 2017
edited
Loading

Copy link
Copy Markdown

Brett Cannon (brettcannon) changed the title Fix issues when running without debuging and debugged code terminates Fix issues when running without debugging and debugged code terminates Nov 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You probably want to read https://www.python.org/dev/peps/pep-0008/ 😉

@@ -0,0 +1,150 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
"""Run a block of code or Python file."""


import sys
import os
from os import path

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

import os.path.

import visualstudio_py_util as _vspu
except:
traceback.print_exc()
print('''Internal error detected. Please copy the above traceback and report at

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Use """.

LOAD = _vspu.to_bytes('LOAD')

def launch():
# Arguments are:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should be in a docstring.

# 6. Startup script name.
# 7. Script arguments.

# change to directory we expected to start from.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

"Change" (didn't capitalize the comments).

return [this.args.program].concat(programArgs);
protected getLauncherFilePath(): string {
const currentFileName = module.filename;
const ptVSToolsPath = path.join(path.dirname(currentFileName), '..', '..', '..', '..', 'pythonFiles', 'PythonTools');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is there no better way to get to the extension's directory than manually walking backwards?

Don Jayamanne (DonJayamanne) Nov 16, 2017
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No, the extension directory is exposed to the extension, but the debugger is launched in a separate process and we don't have access to the same information that the extension has.

sys.stderr.write(out)
sys.stderr.flush()

def is_same_py_file(file1, file2):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

file_1, file_2 or something as I barely noticed the two different variable names below with the numbers so close to the text.

// tslint:disable-next-line:member-ordering
protected handleProcessOutput(proc: ChildProcess, failedToLaunch: (error: Error | string | Buffer) => void) {
proc.on('error', error => {
// TODO: This condition makes no sense (refactor)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

How bad would it be to fix this now since we all know TODOs never get done. 😉

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

agreed

Comment thread src/client/debugger/Main.ts Outdated
}
// If launching the integrated terminal is not supported, then defer to external terminal
// If launching the integrated terminal is not supported, then defer to external terminal
// that will be displayed by our own code

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Missing a period.

Comment thread src/client/debugger/Main.ts Outdated
}
else {
this.pythonProcess.SendResumeThread(pyThread.Id);
if (this.launchArgs.noDebug !== true) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wouldn't this be !this.launchArgs.noDebug, or does this have to do with null/undefined? Same with the explicit comparisons against boolean values below.

Copy link
Copy Markdown
Author

Brett Cannon (@brettcannon) all fixed.

Copy link
Copy Markdown
Author

Brett Cannon (@brettcannon) , I came across a bug when running on Mac OS (I've fixed the issue with 09b3359 and 9616eef, its the last two commit in this PR) and also filed an issue in the vscode repo.

Copy link
Copy Markdown
Author

Brett Cannon (@brettcannon) all comments have been resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor refactoring to do, but I trust you to do it without me blocking you on another round of review.

4. Debug options (not used).
5. '-m' or '-c' to override the default run-as mode. [optional].
6. Startup script name.
7. Script arguments

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Either add a . or drop them all from the list.

LOAD = _vspu.to_bytes('LOAD')

def launch():
"""Arguments are:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add a one-sentence explanation of what this function does.

OUTP = _vspu.to_bytes('OUTP')
LOAD = _vspu.to_bytes('LOAD')

def launch():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe rename to parse_argv() that teases out the details and then drop the call to run()? Then in the __name__ == '__main__' can call parse_argv() and then call run()?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Totally agreed, that's cleaner (SOC)

Don Jayamanne (DonJayamanne) merged commit a6b49d2 into microsoft:master Nov 22, 2017
Don Jayamanne (DonJayamanne) deleted the RunWithoutDebug branch December 12, 2017 21:22
Don Jayamanne (DonJayamanne) added a commit that referenced this pull request Dec 14, 2017
* 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)
lock Bot locked as resolved and limited conversation to collaborators Jul 31, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Back | FazBrowse Home | New Git URL