Repository navigation
Standardize interpreter display format - #2540
Don Jayamanne (DonJayamanne) merged 27 commits into
Conversation
Don Jayamanne (DonJayamanne)
changed the title
| export class InterpreterComparer implements IInterpreterComparer { | ||
| private readonly interpreterHelper: IInterpreterHelper; | ||
|
|
||
| constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { |
There was a problem hiding this comment.
Can we just use the minimal interface required here?
There was a problem hiding this comment.
(If we only need IInterpreterHelper, why not just add that to the constructor?)
| constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { | ||
| this.interpreterHelper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper); | ||
| } | ||
| public compare(a: PythonInterpreter, b: PythonInterpreter): number { |
There was a problem hiding this comment.
What happens when they are equal?
I would expect (in a canonical sense, perhaps) that a compare method would return 1, 0, -1.
Maybe a rename will clear this up? Such as isGreaterThan or something such as that.
| if (info.version_info && info.version_info.length > 0) { | ||
| sortNameParts.push(info.version_info.slice(0, 3).join('.')); | ||
| } | ||
| if (info.version_info) { |
There was a problem hiding this comment.
Perhaps this is meant to check if info.architecture is undefined?
78eae0e to
7f7379c
Compare
Derek Keeler (d3r3kk)
left a comment
Derek Keeler (d3r3kk)
left a comment
There was a problem hiding this comment.
Looks good, let's take time to discuss my idea about revising the pattern we currently use for identifying virtual environments, not a blocker for this PR.
I like the change and think it moves us forward in the right direction!
| } | ||
| }); | ||
| if (!interpreterExists && !details && interpreters.length > 0) { | ||
| this.statusBar.tooltip = ''; |
There was a problem hiding this comment.
Might be something we can use to alert the newer users that they need to pick one of these, and give them some guidance? Might be something we can add to the 'getting started' project.
|
Re-run Azure CI |
2 participants
Fixes #1351
package.jsonare pinned (e.g."1.2.3", not"^1.2.3"for the specified version)package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed)