Skip to content

Commit ecfc767

Browse files
andyleejordanandyleejordan
committed
Use GitCommitId to avoid pointless update checks
And remove an unused `IRunspaceDetails` interface.
1 parent 2698ac1 commit ecfc767

File tree

3 files changed

+119
-101
lines changed

3 files changed

+119
-101
lines changed

‎src/features/UpdatePowerShell.ts

+21-3
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ export class UpdatePowerShell {
5353
private sessionSettings: Settings,
5454
private logger: ILogger,
5555
versionDetails: IPowerShellVersionDetails) {
56-
this.localVersion = new SemVer(versionDetails.version);
56+
// We use the commit field as it's like
57+
// '7.3.0-preview.3-508-g07175ae0ff8eb7306fe0b0fc7d...' which translates
58+
// to SemVer. The version handler in PSES handles Windows PowerShell and
59+
// just returns the first three fields like '5.1.22621'.
60+
this.localVersion = new SemVer(versionDetails.commit);
5761
this.architecture = versionDetails.architecture.toLowerCase();
5862
}
5963

@@ -77,8 +81,22 @@ export class UpdatePowerShell {
7781
return false;
7882
}
7983

80-
// TODO: Check if PowerShell is self-built, i.e. PSVersionInfo.GitCommitId.Length > 40.
81-
// TODO: Check if preview is a daily build.
84+
if (this.localVersion.prerelease.length > 1) {
85+
const prerelease = this.localVersion.prerelease[1].toString();
86+
87+
// Skip if PowerShell is self-built, that is, this contains a commit hash.
88+
if (prerelease.length >= 40) {
89+
this.logger.writeDiagnostic("Not offering to update development build.");
90+
return false;
91+
}
92+
93+
// Skip if preview is a daily build.
94+
if (prerelease.toLowerCase().match("daily")) {
95+
this.logger.writeDiagnostic("Not offering to update daily build.");
96+
return false;
97+
}
98+
}
99+
82100
// TODO: Check if network is available?
83101
// TODO: Only check once a week.
84102
this.logger.writeDiagnostic("Should check for PowerShell update.");

‎src/session.ts

+6-14
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
OperatingSystem, PowerShellExeFinder
2525
} from "./platform";
2626
import { LanguageClientConsumer } from "./languageClientConsumer";
27+
import { SemVer } from "semver";
2728

2829
export enum SessionStatus {
2930
NeverStarted,
@@ -54,17 +55,11 @@ export interface IEditorServicesSessionDetails {
5455

5556
export interface IPowerShellVersionDetails {
5657
version: string;
57-
displayVersion: string;
5858
edition: string;
59+
commit: string;
5960
architecture: string;
6061
}
6162

62-
export interface IRunspaceDetails {
63-
powerShellVersion: IPowerShellVersionDetails;
64-
runspaceType: RunspaceType;
65-
connectionString: string;
66-
}
67-
6863
export type IReadSessionFileCallback = (details: IEditorServicesSessionDetails) => void;
6964

7065
export const SendKeyPressNotificationType =
@@ -734,12 +729,9 @@ Type 'help' to get help.
734729
this.languageStatusItem.detail = "PowerShell";
735730

736731
if (this.versionDetails !== undefined) {
737-
const version = this.versionDetails.architecture.toLowerCase() !== "x64"
738-
? `${this.versionDetails.displayVersion} (${this.versionDetails.architecture.toLowerCase()})`
739-
: this.versionDetails.displayVersion;
740-
741-
this.languageStatusItem.text = "$(terminal-powershell) " + version;
742-
this.languageStatusItem.detail += " " + version;
732+
const semver = new SemVer(this.versionDetails.version);
733+
this.languageStatusItem.text = `$(terminal-powershell) ${semver.major}.${semver.minor}`;
734+
this.languageStatusItem.detail += ` ${this.versionDetails.commit} (${this.versionDetails.architecture.toLowerCase()})`;
743735
}
744736

745737
if (statusText) {
@@ -835,7 +827,7 @@ Type 'help' to get help.
835827
const powerShellSessionName =
836828
currentPowerShellExe ?
837829
currentPowerShellExe.displayName :
838-
`PowerShell ${this.versionDetails.displayVersion} ` +
830+
`PowerShell ${this.versionDetails.version} ` +
839831
`(${this.versionDetails.architecture.toLowerCase()}) ${this.versionDetails.edition} Edition ` +
840832
`[${this.versionDetails.version}]`;
841833

‎test/features/UpdatePowerShell.test.ts

+92-84
Original file line numberDiff line numberDiff line change
@@ -21,95 +21,103 @@ describe("UpdatePowerShell feature", function () {
2121
process.env.POWERSHELL_UPDATECHECK = currentUpdateSetting;
2222
});
2323

24-
describe("When it should check for an update", function () {
25-
it("Won't check if 'promptToUpdatePowerShell' is false", function () {
26-
settings.promptToUpdatePowerShell = false;
27-
process.env.POWERSHELL_UPDATECHECK = "Default";
28-
const version: IPowerShellVersionDetails = {
29-
"version": "7.3.0",
30-
"displayVersion": "7.3",
31-
"edition": "Core",
32-
"architecture": "X64"
33-
};
34-
// @ts-expect-error testing doesn't require all arguments.
35-
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
36-
// @ts-expect-error method is private.
37-
assert(!updater.shouldCheckForUpdate());
38-
});
24+
it("Won't check if 'promptToUpdatePowerShell' is false", function () {
25+
settings.promptToUpdatePowerShell = false;
26+
process.env.POWERSHELL_UPDATECHECK = "Default";
27+
const version: IPowerShellVersionDetails = {
28+
"version": "7.3.0",
29+
"edition": "Core",
30+
"commit": "7.3.0",
31+
"architecture": "X64"
32+
};
33+
// @ts-expect-error testing doesn't require all arguments.
34+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
35+
// @ts-expect-error method is private.
36+
assert(!updater.shouldCheckForUpdate());
37+
});
38+
39+
it("Won't check for Windows PowerShell", function () {
40+
const version: IPowerShellVersionDetails = {
41+
"version": "5.1.22621",
42+
"edition": "Desktop",
43+
"commit": "5.1.22621",
44+
"architecture": "X64"
45+
};
46+
// @ts-expect-error testing doesn't require all arguments.
47+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
48+
// @ts-expect-error method is private.
49+
assert(!updater.shouldCheckForUpdate());
50+
});
3951

40-
it("Won't check for Windows PowerShell", function () {
41-
const version: IPowerShellVersionDetails = {
42-
// TODO: This should handle e.g. 5.1.22621.436
43-
"version": "5.1.0",
44-
"displayVersion": "5.1",
45-
"edition": "Desktop",
46-
"architecture": "X64"
47-
};
48-
// @ts-expect-error testing doesn't require all arguments.
49-
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
50-
// @ts-expect-error method is private.
51-
assert(!updater.shouldCheckForUpdate());
52-
});
52+
it("Won't check for a development build of PowerShell", function () {
53+
const version: IPowerShellVersionDetails = {
54+
"version": "7.3.0-preview.3",
55+
"edition": "Core",
56+
"commit": "7.3.0-preview.3-508-g07175ae0ff8eb7306fe0b0fc7d19bdef4fbf2d67",
57+
"architecture": "Arm64"
58+
};
59+
// @ts-expect-error testing doesn't require all arguments.
60+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
61+
// @ts-expect-error method is private.
62+
assert(!updater.shouldCheckForUpdate());
63+
});
5364

54-
it("Won't check if POWERSHELL_UPDATECHECK is 'Off'", function () {
55-
process.env.POWERSHELL_UPDATECHECK = "Off";
56-
const version: IPowerShellVersionDetails = {
57-
"version": "7.3.0",
58-
"displayVersion": "7.3",
59-
"edition": "Core",
60-
"architecture": "X64"
61-
};
62-
// @ts-expect-error testing doesn't require all arguments.
63-
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
64-
// @ts-expect-error method is private.
65-
assert(!updater.shouldCheckForUpdate());
66-
});
65+
it("Won't check if POWERSHELL_UPDATECHECK is 'Off'", function () {
66+
process.env.POWERSHELL_UPDATECHECK = "Off";
67+
const version: IPowerShellVersionDetails = {
68+
"version": "7.3.0",
69+
"edition": "Core",
70+
"commit": "7.3.0",
71+
"architecture": "X64"
72+
};
73+
// @ts-expect-error testing doesn't require all arguments.
74+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
75+
// @ts-expect-error method is private.
76+
assert(!updater.shouldCheckForUpdate());
77+
});
6778

68-
it ("Should otherwise check to update PowerShell", function () {
69-
process.env.POWERSHELL_UPDATECHECK = "Default";
70-
const version: IPowerShellVersionDetails = {
71-
"version": "7.3.0",
72-
"displayVersion": "7.3",
73-
"edition": "Core",
74-
"architecture": "X64"
75-
};
76-
// @ts-expect-error testing doesn't require all arguments.
77-
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
78-
// @ts-expect-error method is private.
79-
assert(updater.shouldCheckForUpdate());
80-
});
79+
it("Should otherwise check to update PowerShell", function () {
80+
process.env.POWERSHELL_UPDATECHECK = "Default";
81+
const version: IPowerShellVersionDetails = {
82+
"version": "7.3.0",
83+
"edition": "Core",
84+
"commit": "7.3.0",
85+
"architecture": "X64"
86+
};
87+
// @ts-expect-error testing doesn't require all arguments.
88+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
89+
// @ts-expect-error method is private.
90+
assert(updater.shouldCheckForUpdate());
8191
});
8292

83-
describe("Which version it gets", function () {
84-
it("Would update to LTS", async function() {
85-
process.env.POWERSHELL_UPDATECHECK = "LTS";
86-
const version: IPowerShellVersionDetails = {
87-
"version": "7.0.0",
88-
"displayVersion": "7.0",
89-
"edition": "Core",
90-
"architecture": "X64"
91-
};
92-
// @ts-expect-error testing doesn't require all arguments.
93-
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
94-
// @ts-expect-error method is private.
95-
const tag: string | undefined = await updater.maybeGetNewRelease();
96-
// NOTE: This will need to be updated each new major LTS.
97-
assert(tag?.startsWith("v7.2"));
98-
});
93+
it("Would update to LTS", async function () {
94+
process.env.POWERSHELL_UPDATECHECK = "LTS";
95+
const version: IPowerShellVersionDetails = {
96+
"version": "7.0.0",
97+
"edition": "Core",
98+
"commit": "7.0.0",
99+
"architecture": "X64"
100+
};
101+
// @ts-expect-error testing doesn't require all arguments.
102+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
103+
// @ts-expect-error method is private.
104+
const tag: string | undefined = await updater.maybeGetNewRelease();
105+
// NOTE: This will need to be updated each new major LTS.
106+
assert(tag?.startsWith("v7.2"));
107+
});
99108

100-
it("Would update to stable", async function() {
101-
const version: IPowerShellVersionDetails = {
102-
"version": "7.0.0",
103-
"displayVersion": "7.0",
104-
"edition": "Core",
105-
"architecture": "X64"
106-
};
107-
// @ts-expect-error testing doesn't require all arguments.
108-
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
109-
// @ts-expect-error method is private.
110-
const tag: string | undefined = await updater.maybeGetNewRelease();
111-
// NOTE: This will need to be updated each new major stable.
112-
assert(tag?.startsWith("v7.3"));
113-
});
109+
it("Would update to stable", async function () {
110+
const version: IPowerShellVersionDetails = {
111+
"version": "7.0.0",
112+
"edition": "Core",
113+
"commit": "7.0.0",
114+
"architecture": "X64"
115+
};
116+
// @ts-expect-error testing doesn't require all arguments.
117+
const updater = new UpdatePowerShell(undefined, settings, testLogger, version);
118+
// @ts-expect-error method is private.
119+
const tag: string | undefined = await updater.maybeGetNewRelease();
120+
// NOTE: This will need to be updated each new major stable.
121+
assert(tag?.startsWith("v7.3"));
114122
});
115123
});

0 commit comments

Comments
 (0)