Add delay for exec in terminal #592
Conversation
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)
* upstream/master: Common execution for python tool or module (#468)
* upstream/master: Complete refactoring of code to use new code exec engine (#493)
* upstream/master: Improve interpreter selection on different platforms (#517) Yarn and code coverage (#475)
* upstream/master: Remove Homebrew installation (#537) Introduce per user installation of modules and elevated global install as an option (#532) Fix spawn args (#489)
* upstream/master: Add `pudb` set trace statement to snippets (#370)
* upstream/master: Fix auto-indent regex rules (#562) Fix linter installation (#557) Upload buillds to azure (#534) Delete package-lock.json file (#550) Fix exception reported in #447 (#536)
DonJayamanne
changed the title
Codecov Report
@@ Coverage Diff @@
## master #592 +/- ##
==========================================
+ Coverage 56.29% 58.06% +1.77%
==========================================
Files 212 225 +13
Lines 9975 10152 +177
Branches 1754 1751 -3
==========================================
+ Hits 5615 5895 +280
+ Misses 4355 4252 -103
Partials 5 5
Continue to review full report at Codecov.
|
|
@MikhailArkhipov please could you review this. |
| import { IConfigurationService, IPythonSettings } from '../types'; | ||
|
|
||
| @injectable() | ||
| export class ConfigurationService implements IConfigurationService { |
There was a problem hiding this comment.
Ha-ha I added my own. Will use yours then.
| const executable = command.indexOf(' ') ? `"${command}"` : command; | ||
| const commandPrefix = this.terminalIsPowershell() ? '& ' : ''; | ||
| return `${commandPrefix}${executable} ${args.join(' ')}`.trim(); | ||
| public async sendCommand(command: string, args: string[]): Promise<void> { |
There was a problem hiding this comment.
Just something to discuss... Should we name async functions with Async suffix? It is usually easier to see if you have to await for the call. I've already fixed couple of places that didn't actually await...
There was a problem hiding this comment.
I can understand the need for this in C#.
In TypeScript I wouldn't want to use this async suffix.
Also the compiler won't let you access the result of a async method, due to strong typing.
Besides most code bases in nodejs and typescript don't do this.
Also, async in nodejs is very common and natural (with the return of Promises), i.e. its very common due to the nature of the language. Where as C# people are generally used to writing everything sync, only in the recent past has async become more main stream (thanks to nodejs).
I've already fixed couple of places that didn't actually await
Thats poor coding (yes my own code). At a minimjum we should add a catch for the promises.
But, still lets discuss this.
There was a problem hiding this comment.
In hindsight I should i picked on this when reviewing your PR. damn.
| const IS_FISH = /(fish$)/i; | ||
|
|
||
| @injectable() | ||
| export class TerminalHelper implements ITerminalHelper { |
There was a problem hiding this comment.
Technically can just be part of the TerminalService unless it used elsewhere separately.
There was a problem hiding this comment.
Will fix in the other branch.
Fixes #174
Fixes #60
Fixes #593