Make preset reloading less aggressive: gate on configureOnEdit and dirty preset files (#4793)

* Initial plan

* Make preset reloading less aggressive: only reapply when configureOnEdit is true and preset files had unsaved changes

Co-authored-by: snehara99 <[email protected]>

* Restore .npmrc and yarn.lock accidentally modified during build

Co-authored-by: snehara99 <[email protected]>

* Add clarifying comments for _referencedFiles population in presetsController

Co-authored-by: snehara99 <[email protected]>

* Add changelog entry for issue #4792

Co-authored-by: snehara99 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: snehara99 <[email protected]>

Author: Copilot

CHANGELOG.md

@@ -31,6 +31,7 @@ Bug Fixes:
 - Fix bookmarked targets running a full build instead of building only the specific target when triggered from the Bookmarks view. [#4771](https://github.com/microsoft/vscode-cmake-tools/issues/4771)
 - Fix `cmake.compileFile` command truncating long compile commands at ~1024 characters on macOS. The command is now sent to the terminal in chunks to avoid VS Code terminal buffer limitations. [#4470](https://github.com/microsoft/vscode-cmake-tools/issues/4470)
 - Fix configure/build sometimes using stale preset values when unsaved changes to included preset files are auto-saved before configure. The extension now explicitly refreshes presets from disk after saving, instead of relying solely on the asynchronous file watcher. [#4502](https://github.com/microsoft/vscode-cmake-tools/issues/4502)
+- Fix overly aggressive preset reloading on every build/debug/launch. Preset reapplication now only runs when `cmake.configureOnEdit` is enabled and preset files had unsaved changes, instead of unconditionally re-reading all preset files. [#4792](https://github.com/microsoft/vscode-cmake-tools/issues/4792)
 - Reduce overly verbose logging when CMake configure or build fails. The Output panel no longer floods with duplicated output, and the channel is only revealed on error rather than unconditionally. [#4749](https://github.com/microsoft/vscode-cmake-tools/issues/4749)
 - Fix Test Results panel not hyperlinking file paths for GoogleTest failures. The default `cmake.ctest.failurePatterns` now includes a pattern matching GoogleTest's `file:line: Failure` output format. [#4589](https://github.com/microsoft/vscode-cmake-tools/issues/4589)
 - Fix wrong path created for artifact when parsing code model using CMake File API. [#3015](https://github.com/microsoft/vscode-cmake-tools/issues/3015)

src/cmakeProject.ts

@@ -1843,7 +1843,34 @@ export class CMakeProject {
      * Save all open files. "maybe" because the user may have disabled auto-saving
      * with `config.saveBeforeBuild`.
      */
-    async maybeAutoSaveAll(showCommandOnly?: boolean): Promise<boolean> {
+    /**
+     * Save all open files. "maybe" because the user may have disabled auto-saving
+     * with `config.saveBeforeBuild`.
+     *
+     * When preset files have unsaved changes, this method also:
+     * 1. Suppresses the file-watcher to avoid redundant reapplyPresets() calls
+     * 2. Saves all open editors
+     * 3. Conditionally reapplies presets from disk so the caller sees fresh state
+     * 4. Resumes the file-watcher
+     *
+     * @param showCommandOnly When true, skip the "saving files" log message
+     * @param requireConfigureOnEdit When true (default), dirty presets are only
+     *   reapplied if cmake.configureOnEdit is also enabled.  Pass false for
+     *   explicit-configure paths where the user expects presets to be refreshed
+     *   regardless of that setting (see #4792).
+     */
+    async maybeAutoSaveAll(showCommandOnly?: boolean, requireConfigureOnEdit: boolean = true): Promise<boolean> {
+        // Snapshot dirty preset state *before* saving so we can report it back.
+        // After saveAll() the dirty flags will be cleared.
+        const hadDirtyPresets = this.useCMakePresets && this.haveUnsavedPresetFileChanges();
+
+        // Suppress watcher-triggered reapply during save — we will explicitly
+        // await reapplyPresets() below when needed, so the file-watcher's
+        // fire-and-forget call would be redundant (see #4792).
+        if (hadDirtyPresets) {
+            this.presetsController.suppressWatcherReapply = true;
+        }
+
         // Save open files before we configure/build
         if (this.workspaceContext.config.saveBeforeBuild) {
             if (!showCommandOnly) {
@@ -1881,28 +1908,58 @@ export class CMakeProject {
                 if (chosen?.title === yesAndDoNotShowAgain) {
                     await cmakeConfiguration.update(showSaveFailedNotificationString, false, vscode.ConfigurationTarget.Global);
                 }
-                return chosen !== undefined && (chosen.title === yesButtonTitle || chosen.title === yesAndDoNotShowAgain);
+                const saved = chosen !== undefined && (chosen.title === yesButtonTitle || chosen.title === yesAndDoNotShowAgain);
+                if (!saved) {
+                    this.presetsController.suppressWatcherReapply = false;
+                    return false;
+                }
             }
         }
+
+        // After saving, explicitly refresh presets from disk so that any
+        // just-saved changes are picked up before configure/build runs.
+        // Without this, the async file-watcher may not have completed yet
+        // (see #4502).  The configureOnEdit gate is skipped for explicit-
+        // configure paths (requireConfigureOnEdit=false) — see #4792.
+        if (hadDirtyPresets && (!requireConfigureOnEdit || this.workspaceContext.config.configureOnEdit)) {
+            await this.presetsController.reapplyPresets();
+        }
+        // Resume normal file-watcher behavior now that the explicit reapply
+        // (if any) has completed.  This is a no-op when hadDirtyPresets was false.
+        this.presetsController.suppressWatcherReapply = false;
+
         return true;
     }
 
+    /**
+     * Returns true when at least one of the preset files tracked by the
+     * presets controller (including files pulled in via "include") is currently
+     * dirty (has unsaved changes) in VS Code.
+     */
+    private haveUnsavedPresetFileChanges(): boolean {
+        const dirtyPaths = new Set(
+            vscode.workspace.textDocuments
+                .filter(doc => doc.isDirty)
+                .map(doc => util.platformNormalizePath(doc.uri.fsPath))
+        );
+        return this.presetsController.referencedFiles
+            .some(f => dirtyPaths.has(util.platformNormalizePath(f)));
+    }
+
     /**
      * Wraps pre/post configure logic around an actual configure function
      * @param cb The actual configure callback. Called to do the configure
      */
     private async doConfigure(type: ConfigureType, progress: ProgressHandle, cb: (consumer: CMakeOutputConsumer) => Promise<ConfigureResult>): Promise<ConfigureResult> {
         progress.report({ message: localize('saving.open.files', 'Saving open files') });
-        if (!await this.maybeAutoSaveAll(type === ConfigureType.ShowCommandOnly)) {
+        // configureOnEdit is intentionally NOT required here. doConfigure()
+        // is the explicit configure path — when the user manually triggers
+        // configure, we should always pick up just-saved preset changes
+        // regardless of configureOnEdit (which only controls *automatic*
+        // reconfigure on edit). See #4792.
+        if (!await this.maybeAutoSaveAll(type === ConfigureType.ShowCommandOnly, false)) {
             return { exitCode: -1, resultType: ConfigureResultType.Other };
         }
-        // After saving files, explicitly refresh presets from disk so that any
-        // just-saved changes to preset files (including included files) are picked
-        // up before configure runs.  Without this, the asynchronous file-watcher
-        // may not have re-expanded the presets yet (see #4502).
-        if (this.useCMakePresets) {
-            await this.presetsController.reapplyPresets();
-        }
         if (!this.useCMakePresets) {
             if (!this.activeKit) {
                 await vscode.window.showErrorMessage(localize('cannot.configure.no.kit', 'Cannot configure: No kit is active for this CMake project'));
@@ -1989,18 +2046,10 @@ export class CMakeProject {
         if (!drv) {
             return null;
         }
-        // First, save open files
+        // First, save open files and conditionally reapply dirty presets
         if (!await this.maybeAutoSaveAll()) {
             return { exitCode: -1 };
         }
-        // After saving, explicitly refresh presets from disk so that the
-        // needsReconfigure check below sees up-to-date preset state.
-        // Without this, the async file-watcher's fire-and-forget reapplyPresets()
-        // may not have completed yet, causing needsReconfigure() to return false
-        // even though preset files just changed on disk (see #4502).
-        if (this.useCMakePresets) {
-            await this.presetsController.reapplyPresets();
-        }
         if (await this.needsReconfigure()) {
             return this.configureInternal(ConfigureTrigger.compilation, [], ConfigureType.Normal, undefined, cancellationToken);
         } else {
@@ -2976,9 +3025,6 @@ export class CMakeProject {
         if (!await this.maybeAutoSaveAll()) {
             return null;
         }
-        if (this.useCMakePresets) {
-            await this.presetsController.reapplyPresets();
-        }
 
         // Ensure that we've configured the project already. If we haven't, `getOrSelectLaunchTarget` won't see any
         // executable targets and may show an uneccessary prompt to the user

src/presets/presetsController.ts

@@ -28,9 +28,12 @@ export class PresetsController implements vscode.Disposable {
     private _presetsWatchers: FileWatcher | undefined;
     private _sourceDirChangedSub: vscode.Disposable | undefined;
     private _isChangingPresets = false;
+    // Populated by reapplyPresets() with paths of all preset files (CMakePresets.json,
+    // CMakeUserPresets.json, and any files pulled in via "include").
     private _referencedFiles: string[] = [];
     private _presetsParser!: PresetsParser; // Using definite assigment (!) because we initialize it in the init method
     private _reapplyInProgress: Promise<void> = Promise.resolve();
+    private _suppressWatcherReapply: boolean = false;
 
     private readonly _presetsChangedEmitter = new vscode.EventEmitter<preset.PresetsFile | undefined>();
     private readonly _userPresetsChangedEmitter = new vscode.EventEmitter<preset.PresetsFile | undefined>();
@@ -133,6 +136,21 @@ export class PresetsController implements vscode.Disposable {
         return this._presetsParser.userPresetsPath;
     }
 
+    get referencedFiles(): readonly string[] {
+        return this._referencedFiles;
+    }
+
+    /**
+     * When true, the file-watcher's change handler will not call reapplyPresets().
+     * Set this before saveAll() when the caller will explicitly await reapplyPresets()
+     * afterward, to avoid redundant re-reads triggered by the OS file-change
+     * notification for the same save (see #4792).
+     * Cleared automatically at the end of reapplyPresets().
+     */
+    set suppressWatcherReapply(value: boolean) {
+        this._suppressWatcherReapply = value;
+    }
+
     get workspaceFolder() {
         return this.project.workspaceFolder;
     }
@@ -191,7 +209,8 @@ export class PresetsController implements vscode.Disposable {
                 this.project.workspaceContext.config.allowUnsupportedPresetsVersions
             );
 
-            // reset all expanded presets storage.
+            // Collect the paths of all referenced preset files (main files + includes).
+            // resetPresetsFiles() populates the referencedFiles map as it parses each file.
             this._referencedFiles = Array.from(referencedFiles.keys());
 
             this.project.minCMakeVersion = preset.minCMakeVersion(this.folderPath);
@@ -202,6 +221,10 @@ export class PresetsController implements vscode.Disposable {
             // Don't need to set build/test presets here since they are reapplied in setConfigurePreset
 
             await this.watchPresetsChange();
+
+            // Clear after completing so that late watcher events from a
+            // prior saveAll() remain suppressed for the entire reapply.
+            this._suppressWatcherReapply = false;
         };
         this._reapplyInProgress = this._reapplyInProgress.then(doReapply, doReapply);
         return this._reapplyInProgress;
@@ -1715,7 +1738,9 @@ export class PresetsController implements vscode.Disposable {
     private async watchPresetsChange() {
 
         const presetChangeHandler = () => {
-            void this.reapplyPresets();
+            if (!this._suppressWatcherReapply) {
+                void this.reapplyPresets();
+            }
         };
         const presetCreatedHandler = () => {
             void this.onCreatePresetsFile();