Repository navigation
Check environment path when ext loads - #1982
Conversation
| const message = InvalidEnvPathVariableMessage | ||
| .format(this.platform.pathVariableName, env.extensionName); | ||
| return [new InvalidEnvironmentPathVariableDiagnostic(message)]; | ||
| } else { |
There was a problem hiding this comment.
No love for macOS or Linux? Perhaps their PATH is less likely to be corrupted?
There was a problem hiding this comment.
Yes, we've only seen users on Windows having problems.
| 'use strict'; | ||
|
|
||
| import { inject, injectable } from 'inversify'; | ||
| import { DiagnosticSeverity } from 'vscode'; |
There was a problem hiding this comment.
Maybe not useful to do this, but I'll ask the question anyways:
Could we not just abstract this out to an INT or something?
...I feel like tying our App diagnostic log mechanism doesn't really need to be tied to the vscode app itself. But again, perhaps it's not useful for the extra work it would cause/extra maintenance it might invoke...
There was a problem hiding this comment.
Agreed, I'd leave this for now.
| }, | ||
| { | ||
| prompt: 'More Info', | ||
| command: commandFactory.createCommand(diagnostic, { type: 'launch', options: 'https://github.com/Microsoft/vscode-python/issues/1698' }) |
There was a problem hiding this comment.
Maybe we can make this an aka.ms link instead? That way we can change it to something else if we need to later on.
|
|
||
| import { IDiagnostic, IDiagnosticCommand } from '../types'; | ||
|
|
||
| export abstract class BaseDiagnostCommand implements IDiagnosticCommand { |
There was a problem hiding this comment.
...oh I see consistency in other class names. Perhaps intentional as least characters before collision?
There was a problem hiding this comment.
Lol, it's a typo.
|
|
||
| 'use strict'; | ||
|
|
||
| export enum DiagnosticCodes { |
There was a problem hiding this comment.
Could we use this same pattern to generalize the inclusion of the vscode module's DiagnosticSeverity?
There was a problem hiding this comment.
No, these are error codes. I can use numbers, but chose to use strings instead.
They need to be uniquely identifiable.
There was a problem hiding this comment.
Agreed, and understood - but I am wondering if we can do something like this to remove the use of vscode::DiagnosticSeverity specifically - however we decide to do so... enum, string, number...
There was a problem hiding this comment.
I'm reluctant to do that, might as well use what we have. There's no need to abstract this further just to remove VSC depedency, we've managed to resolve the mocking issue for tests.
|
Derek Keeler (@d3r3kk) Brett Cannon (@brettcannon) |
Codecov Report
@@ Coverage Diff @@
## master #1982 +/- ##
==========================================
+ Coverage 74.42% 74.84% +0.41%
==========================================
Files 287 302 +15
Lines 13484 13693 +209
Branches 2402 2429 +27
==========================================
+ Hits 10036 10249 +213
Misses 3311 3311
+ Partials 137 133 -4
Continue to review full report at Codecov.
|
5f322fd to
f830fa7
Compare
2 participants
Fixes #1698
This pull request: