Don't override include/exclude defaults with false on remove

[yogeshwaran-c]

Summary

Clicking the "Remove" button on a glob in search.exclude (and other include/exclude settings) in the Settings editor was writing "<glob>": false into settings.json when the glob matched a schema default, instead of removing the entry. For search.exclude this broke the documented inheritance from files.exclude.

Fixes #249049.

Details

search.exclude inherits from files.exclude. In getExcludes() the two objects are merged with last-wins semantics, so a false value in search.exclude actively suppresses the true value that would otherwise come from files.exclude.

SettingIncludeExcludeRenderer.onDidChangeIncludeExclude in src/vs/workbench/contrib/preferences/browser/settingsTree.ts treated every non-add event uniformly: if the original key was present in the setting's defaultValue, the renderer wrote false for it ("delete a default by overriding it"). That logic predates the remove/change/move/reset discriminated event types, so it caught the remove case too.

Repro:

1. Open Settings UI for search.exclude. The schema defaults (**/node_modules, **/bower_components, **/*.code-search) are listed.
2. Click the "X" next to **/node_modules.
3. settings.json now contains "search.exclude": { "**/node_modules": false }.
4. Run a workspace search. node_modules is searched even though files.exclude still excludes it.

The fix special-cases e.type === 'remove' and falls through to the existing delete newValue[...] branch, so removing an entry simply deletes it from the user's scope value. change and move still write false over a default - editing a default's text to something else continues to disable the original default, which is the expected behaviour when the user explicitly replaces it.

If a user wants to genuinely disable an inherited default (the previous behaviour of the "Remove" button on a default value), they can still do so by editing settings.json directly.

Test plan

Manual repro in a dev build:

• Empty search.exclude in settings.json. Open Settings UI, click "X" on **/node_modules. Confirm settings.json does not gain a "**/node_modules": false entry, and that the entry disappears from the UI then reappears (it is still a default).
• Add "**/foo": true to search.exclude in settings.json. Open Settings UI, click "X" on **/foo. Confirm the key is removed from settings.json.
• Add "**/foo": true to search.exclude. Open Settings UI, click the edit (pencil) on **/foo and change it to **/bar. Confirm settings.json now has "**/bar": true and no "**/foo" entry.
• In Settings UI, click the edit (pencil) on **/node_modules and change it to **/baz. Confirm settings.json has "**/node_modules": false and "**/baz": true (existing override-default-on-edit behaviour preserved).
• With files.exclude containing "**/node_modules": true, repeat step 1 and run a workspace search; node_modules is correctly excluded.

No automated test added: onDidChangeIncludeExclude is a private renderer method with substantial template/context/IInstantiationService dependencies and there are no existing tests for SettingIncludeExcludeRenderer. Building the harness for a single behavioural assertion would dwarf the fix.

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@rzhao271

Matched files:

• src/vs/workbench/contrib/preferences/browser/settingsTree.ts

[Copilot]

Pull request overview

This PR fixes Settings UI handling for include/exclude glob removal so removing a default glob deletes the scoped setting entry instead of writing a false override that can suppress inherited excludes.

Changes:

• Special-cases remove events in SettingIncludeExcludeRenderer.
• Preserves existing default-override behavior for edit/change scenarios.