Fix confusing download dialog popup #599
Conversation
| @@ -903,7 +905,7 @@ class InstalledTool { | |||
| * @param version Version of the tool, expected to be either SemVer or PVP versioned. | |||
| * @param installed Is this tool currently installed? | |||
| */ | |||
| public constructor(readonly name: string, readonly version: string = '', readonly installed: boolean = true) { | |||
| public constructor(readonly name: string, readonly version?: string, readonly installed: boolean = true) { | |||
| this.nameWithVersion = `${name}-${version}`; | |||
There was a problem hiding this comment.
This will render now as "*-undefined" if it is ever shown which isn't much better, imo.
There was a problem hiding this comment.
Yeah, I don't like this class, because we create an instance even if there is no such version. It obscures control flow. I'll have more time this evening.
There was a problem hiding this comment.
This will render now as "*-undefined" if it is ever shown which isn't much better, imo.
Well, if there's undefined falling through in this codepath, then that's a bug. Because undefined in this context means "default" and should have been resolved before.
There was a problem hiding this comment.
But I also suck hard at OOP, so.
There was a problem hiding this comment.
I don't like this class
Yeah, it is basically a record, not really a class.
Anyway, I prefer it over the unnamed tuple it was before :)
Because undefined in this context means "default" and should have been resolved before.
True
Fixes #597