Standardize interpreter display format #2540
Conversation
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?
DonJayamanne
force-pushed
the
standardInterpreterDisplayFormat2
branch
from
78eae0e to
7f7379c
Compare
| } | ||
| }); | ||
| 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 |
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)