Remove Settings class, use VS Code config API directly

[andyleejordan]

Summary

Replace the cached Settings class hierarchy with direct vscode.workspace.getConfiguration() calls throughout the codebase. This eliminates race conditions from stale cached values and removes the burden of keeping the class in sync with package.json.

Changes

• Remove Settings, DebuggingSettings, DeveloperSettings, etc. classes and all nested settings types
• Remove getSettings(), getSetting(), getEffectiveConfigurationTarget(), onSettingChange(), and onPowerShellSettingChange() helpers
• Remove PowerShellLanguageId constant (inline "powershell" literal)
• Remove unused enums: StartLocation, ExecuteMode, CodeFormattingPreset, PipelineIndentationStyle
• Move CommentType enum to HelpCompletion.ts (its only consumer)
• Replace restartOnCriticalConfigChange() stale-vs-fresh setting comparisons with affectsConfiguration() calls, fixing race conditions
• Replace powerShellDefaultVersion mutation with a dedicated powerShellVersionOverride field
• Audit and fix all setting defaults to match package.json (found mismatches in editorServicesLogLevel and executeMode)
• Disable no-unnecessary-type-arguments ESLint rule (conflicts with no-unnecessary-condition when using VS Code's .get<T>(key, default) API)
• Update tests

Motivation

The Settings class cached configuration values at startup and on change events, but:

1. Race conditions: restartOnCriticalConfigChange() compared stale cached values against fresh reads, which could miss changes
2. Sync burden: Every new setting in package.json required a corresponding field in the class
3. Stale reads: Code checking sessionSettings.foo could get outdated values if the config changed between reads

The VS Code API vscode.workspace.getConfiguration() always returns current values and ConfigurationChangeEvent.affectsConfiguration() reliably detects changes.

[andyleejordan]

This used the new Planning Agent, with the following plan (and maybe an hour or two of review and follow-up fixes and refactors) with Claude Opus 4.6:

Plan: Remove Settings class, use VS Code APIs directly

TL;DR

Remove the Settings class hierarchy from settings.ts and migrate all consumers to use vscode.workspace.getConfiguration() directly. This eliminates the duplicated type definitions (already in package.json), fixes race conditions from stale cached settings, and uses affectsConfiguration() for all change detection.

Key Design Decisions

• Keep helper functions: changeSetting(), validateCwdSetting(), getChosenWorkspace() remain—they wrap VS Code APIs usefully.
• Remove unused helpers: onSettingChange(), onPowerShellSettingChange(), doOnSettingsChange(), onSettingChangeOptions have zero external consumers — remove them. getEffectiveConfigurationTarget() is only used by deprecated migration code — remove both.
• Inline "powershell": Replace all uses of utils.PowerShellLanguageId with the literal "powershell" and remove the constant from utils.ts. The language ID is tied to the package.json contributes.languages entry and will never change.
• Keep enums: CommentType, CodeFormattingPreset, PipelineIndentationStyle, ExecuteMode, StartLocation are used in code logic and don't duplicate package.json.
• Remove getSettings() function and the recursive getSetting() unpacker.
• Remove Settings, PartialSettings, and all nested settings classes (CodeFormattingSettings, DebuggingSettings, etc.).
• Runtime state vs. settings: powerShellDefaultVersion override (when user picks a version from session menu) becomes a separate field on SessionManager, not a mutation of the settings object.
• Change detection: Migrate ALL comparisons in restartOnCriticalConfigChange() to use changeEvent.affectsConfiguration() instead of comparing cached vs. fresh objects (as the existing TODO says).
• Constructor injection removed: SessionManager and PowerShellProcess no longer receive a Settings object. They read what they need at point of use.

Phases

Phase 1: Add thin config helpers to settings.ts

Add a helper function (e.g., getConfigSection(section) or just use inline vscode.workspace.getConfiguration("powershell.section")) to make call sites concise. Potentially a typed getter like getPowerShellSetting<T>(key: string, defaultValue: T): T that wraps getConfiguration("powershell").get<T>(key, defaultValue).

Keep the existing onPowerShellSettingChange() and changeSetting() unchanged.

Files: src/settings.ts

Phase 2: Migrate session.ts

1. Remove sessionSettings: Settings from constructor and all class fields.
2. Remove migrateWhitespaceAroundPipeSetting() and its call site (deprecated codeFormatting.whitespaceAroundPipe → addWhitespaceAroundPipe migration).
3. Add private powerShellVersionOverride: string | undefined to track the runtime version override (currently done via this.sessionSettings.powerShellDefaultVersion = exeNameOverride).
4. Replace restartOnCriticalConfigChange() comparisons:
   • Remove the const settings = getSettings() call
   • Convert ALL 8 property comparisons to changeEvent.affectsConfiguration("powershell.<key>") calls, matching the pattern already used for the cold restart settings
   • This eliminates the race condition entirely
5. Replace all this.sessionSettings.X reads with direct getConfiguration("powershell") or getConfiguration("powershell.section") calls at point of use:
   • this.sessionSettings.enableProfileLoading → getConfiguration("powershell").get<boolean>("enableProfileLoading", true)
   • this.sessionSettings.integratedConsole.suppressStartupBanner → getConfiguration("powershell.integratedConsole").get<boolean>("suppressStartupBanner", false)
   • this.sessionSettings.powerShellAdditionalExePaths → getConfiguration("powershell").get<Record<string, string>>("powerShellAdditionalExePaths", {})
   • this.sessionSettings.powerShellDefaultVersion → check this.powerShellVersionOverride first, then fall back to getConfiguration("powershell").get<string>("powerShellDefaultVersion", "")
6. Remove this.sessionSettings = getSettings() from restartSession().
7. Remove passing this.sessionSettings to PowerShellProcess constructor.

Files: src/session.ts
Depends on: Phase 1

Phase 3: Migrate process.ts

1. Remove sessionSettings: Settings from constructor parameter.
2. Replace all settings reads with direct config access at point of use:
   • this.sessionSettings.developer.featureFlags → getConfiguration("powershell.developer").get<string[]>("featureFlags", [])
   • this.sessionSettings.integratedConsole.useLegacyReadLine → getConfiguration("powershell.integratedConsole").get<boolean>("useLegacyReadLine", false)
   • this.sessionSettings.startAsLoginShell.osx → getConfiguration("powershell.startAsLoginShell").get<boolean>("osx", true)
   • this.sessionSettings.startAsLoginShell.linux → getConfiguration("powershell.startAsLoginShell").get<boolean>("linux", false)
   • this.sessionSettings.developer.setExecutionPolicy → getConfiguration("powershell.developer").get<boolean>("setExecutionPolicy", true)
   • this.sessionSettings.integratedConsole.startInBackground → etc.
   • this.sessionSettings.integratedConsole.startLocation → etc.
   • this.sessionSettings.integratedConsole.showOnStartup → etc.

Files: src/process.ts
Depends on: Phase 1

Phase 4: Migrate feature files (parallelizable)

Each feature file can be migrated independently:

• src/extension.ts: Remove getSettings() call (L61), read startAutomatically directly.
• src/platform.ts: Already uses getSettings() for single field reads → replace with direct config.
• src/features/UpdatePowerShell.ts: Remove sessionSettings: Settings from constructor, read promptToUpdatePowerShell directly. Already uses changeSetting() which stays.
• src/features/HelpCompletion.ts: Remove stored this.settings field, read helpCompletion directly each time. Keep CommentType enum import.
• src/features/Console.ts: Replace getSettings().integratedConsole.focusConsoleOnExecute with direct config read.
• src/features/DebugSession.ts: Replace getSettings().debugging.* with direct config reads.
• src/features/GetCommands.ts: Replace getSettings().sideBar.CommandExplorerExcludeFilter with direct config read.
• src/features/ExtensionCommands.ts: Replace getSettings().integratedConsole.forceClearScrollbackBuffer with direct config read. validateCwdSetting() stays.
• src/features/PesterTests.ts: Replace getSettings().pester.* and getSettings().debugging.* with direct config reads.

Files: All feature files listed above
Depends on: Phase 1

Phase 5: Remove Settings class from settings.ts

1. Remove PartialSettings base class.
2. Remove Settings class and all nested settings classes.
3. Remove getSettings() and getSetting() functions.
4. Remove PowerShellAdditionalExePathSettings type (use Record<string, string> inline or a simple type alias if needed by platform.ts).
5. Remove unused helpers: onSettingChange(), onPowerShellSettingChange(), doOnSettingsChange(), onSettingChangeOptions, getEffectiveConfigurationTarget().
6. Remove PowerShellLanguageId constant from utils.ts (already inlined as "powershell" in earlier phases).
7. Keep: enums (CommentType, CodeFormattingPreset, PipelineIndentationStyle, ExecuteMode, StartLocation), helper functions (changeSetting, validateCwdSetting, getChosenWorkspace).

Files: src/settings.ts
Depends on: Phases 2, 3, 4

Relevant Files

• src/settings.ts — Remove Settings class hierarchy; keep helper functions and enums
• src/session.ts — Remove dependency injection; use direct config reads; fix change detection to use affectsConfiguration() exclusively; add powerShellVersionOverride field
• src/process.ts — Remove sessionSettings constructor param; read config directly
• src/extension.ts — Replace getSettings() with a single getConfiguration call
• src/platform.ts — Replace getSettings() calls with direct config reads
• src/features/UpdatePowerShell.ts — Remove constructor injection
• src/features/HelpCompletion.ts — Remove cached settings field
• src/features/Console.ts — Replace getSettings() call
• src/features/DebugSession.ts — Replace getSettings() calls
• src/features/GetCommands.ts — Replace getSettings() call
• src/features/ExtensionCommands.ts — Replace getSettings() call
• src/features/PesterTests.ts — Replace getSettings() calls

Verification

1. npm run compile — Ensure TypeScript compiles with no errors after each phase
2. npm run lint — Lint passes
3. Run the extension in Extension Development Host, change settings, verify:
   • Settings changes prompt for restart when appropriate
   • PowerShell version switching via session menu works
   • Debugging settings take effect
   • Code formatting settings forwarded to server correctly (via configurationSection sync)
   • Pester code lens respects settings
   • cwd validation still works
4. Existing tests: Invoke-Build Test (client tests)

Scope

• Included: All client-side settings usage migration; change detection fix; constructor injection removal
• Excluded: Server-side settings (handled via LSP configurationSection sync already); package.json changes; the configurationSection sync mechanism itself
• Note: The configurationSection: ["powershell", "files", "search"] sync in LSP client options means the server already pulls settings via the VS Code API protocol — no class needed for that

Design Decisions (confirmed)

• No helper function: Use vscode.workspace.getConfiguration() inline everywhere (no getPowerShellConfig() wrapper).
• Default values: For booleans, get<boolean>("key") without a default is fine — truthy/falsy checks handle undefined naturally. For strings, numbers, arrays, and objects, use get<T>("key", defaultValue) so the return type is T (not T | undefined), avoiding ! or ?? assertions.
• CodeFormattingSettings: The ~20 formatting settings are sent to the language server via configurationSection sync. No client-side code reads them individually, so nothing to migrate.

[Copilot]

Pull request overview

This PR removes the cached Settings class hierarchy and migrates the extension to read/update configuration via vscode.workspace.getConfiguration() directly, aiming to avoid stale cached values and simplify keeping settings in sync with package.json.

Changes:

• Replaced Settings object usage across runtime/features with direct VS Code configuration reads/updates.
• Updated session restart detection to rely on ConfigurationChangeEvent.affectsConfiguration() instead of stale-vs-fresh comparisons.
• Updated E2E/unit tests and adjusted ESLint configuration to accommodate the new .get<T>(key, default) usage pattern.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/features/UpdatePowerShell.test.ts	Updates tests to toggle VS Code config instead of mutating a Settings instance.
test/core/settings.test.ts	Removes Settings/helper-based assertions; validates config updates via vscode.workspace.getConfiguration().
src/utils.ts	Removes PowerShellLanguageId constant (call sites now inline "powershell").
src/settings.ts	Removes the Settings model + helpers; keeps changeSetting, getChosenWorkspace, and validateCwdSetting using VS Code config API.
src/session.ts	Removes stored Settings dependency; reads config on demand; refactors restart logic and PowerShell version selection.
src/process.ts	Removes Settings injection; reads integrated console + developer settings directly from configuration.
src/platform.ts	Uses config API directly for suppression setting; simplifies additional-exe-paths type.
src/features/UpdatePowerShell.ts	Reads promptToUpdatePowerShell directly from configuration.
src/features/PesterTests.ts	Reads debugging/pester settings via configuration objects.
src/features/HelpCompletion.ts	Moves CommentType locally and reads helpCompletion from configuration instead of cached settings.
src/features/GetCommands.ts	Reads sidebar exclusion filter directly from configuration.
src/features/ExtensionCommands.ts	Reads console “force clear scrollback” directly from configuration.
src/features/DebugSession.ts	Resolves debug defaults directly from configuration.
src/features/Console.ts	Reads focusConsoleOnExecute directly from configuration.
src/extension.ts	Removes initial settings load/log; uses config directly for language id and auto-start.
eslint.config.mjs	Disables @typescript-eslint/no-unnecessary-type-arguments to avoid conflicts with VS Code config .get<T>(..., default) patterns.

Comments suppressed due to low confidence (2)

src/session.ts:363

• powerShellVersionOverride is set when restartSession() is called with exeNameOverride, but it is never cleared when restartSession() is called without an override. After a single overridden restart (e.g. from the session menu), subsequent plain restarts will continue to prefer the old override over the configured powershell.powerShellDefaultVersion, which differs from the prior behavior when settings were reloaded each restart. Consider clearing powerShellVersionOverride when no override is requested (or explicitly making/communicating the override as sticky by design).

    private async restartSession(exeNameOverride?: string): Promise<void> {
        this.logger.write("Restarting session...");
        await this.stop();

        if (exeNameOverride) {
            // Reset the version and PowerShell details since we're launching a
            // new executable.
            this.logger.writeDebug(
                `Starting with executable overriden to: ${exeNameOverride}`,
            );
            this.powerShellVersionOverride = exeNameOverride;
            this.versionDetails = undefined;
            this.PowerShellExeDetails = undefined;
        }

        await this.start();
    }

src/session.ts:719

• In the warning when the requested PowerShell display name isn't found, the message hard-codes "The 'powerShellDefaultVersion' setting was ...". After introducing powerShellVersionOverride, wantedName may come from the override rather than the setting, so this message can be misleading. Consider rewording to refer to the requested/selected PowerShell name (and optionally mention whether it came from the override vs configuration).

            wantedName ??=
                this.powerShellVersionOverride ??
                vscode.workspace
                    .getConfiguration("powershell")
                    .get("powerShellDefaultVersion", "");
            if (wantedName !== "") {
                for await (const details of powershellExeFinder.enumeratePowerShellInstallations()) {
                    // Need to compare names case-insensitively, from https://stackoverflow.com/a/2140723
                    if (
                        wantedName.localeCompare(
                            details.displayName,
                            undefined,
                            { sensitivity: "accent" },
                        ) === 0
                    ) {
                        defaultPowerShell = details;
                        break;
                    }
                }
            }
            foundPowerShell =
                defaultPowerShell ??
                (await powershellExeFinder.getFirstAvailablePowerShellInstallation());
            if (
                wantedName !== "" &&
                defaultPowerShell === undefined &&
                foundPowerShell !== undefined
            ) {
                void this.logger.writeAndShowWarning(
                    `The 'powerShellDefaultVersion' setting was '${wantedName}' but this was not found!` +
                        ` Instead using first available installation '${foundPowerShell.displayName}' at '${foundPowerShell.exePath}'!`,
                );

[Copilot]

package.json still marks powershell.codeFormatting.whitespaceAroundPipe as deprecated and says the extension will automatically migrate it to codeFormatting.addWhitespaceAroundPipe, but the migration logic (migrateWhitespaceAroundPipeSetting) and its invocation were removed. This means existing users with the deprecated setting set will no longer be migrated as promised. Consider reinstating the migration (or updating the package.json deprecation text if migration is intentionally removed).

[Copilot]

Spelling in the debug log message: "overriden" should be "overridden".

Suggested change

	`Starting with executable overriden to: ${exeNameOverride}`,
	`Starting with executable overridden to: ${exeNameOverride}`,