Skip to content

Preserve literal dollars in case-converted search replacements - #334710

Open
Suzu (Suzu1Dev) wants to merge 1 commit into
microsoft:mainfrom
Suzu1Dev:fix/search-replace-literal-dollars
Open

Suzu (Suzu1Dev) wants to merge 1 commit into
microsoft:mainfrom
Suzu1Dev:fix/search-replace-literal-dollars

Conversation

@Suzu1Dev

Copy link
Copy Markdown

Workspace Search reinterprets dollar sequences in case-converted captures as replacement syntax. Searching ^(foo.*)$ in foo$1 and replacing with \U$1 currently produces FOOfoo$1; this change produces FOO$1 in both the preview and the saved file.

Fixes #334709.

Collect the converted capture separately and escape its literal dollars before appending it to the replacement template. Other capture substitutions continue through the existing path. Add seven regression cases covering capture references, whole-match/prefix/suffix references, escaped dollars, named-reference text, and mixed converted/unconverted captures.

Related to #299365 and #326451, which handle $$ entered in the replacement field. That PR does not cover dollar sequences in case-converted captured text. This branch is based directly on main and does not depend on #334698.

Validation

On macOS arm64 with Node 24.18.0:

  • All seven new regression cases fail on the unmodified implementation and pass with this change.
  • 258 related Electron unit tests pass; three existing/platform-specific tests are skipped.
  • Changed-file ESLint, hygiene, and git diff --check pass.
  • With fresh profiles and --disable-extensions, verified the Search view preview and actual Replace All edits in both baseline and fixed Code OSS: foo$1, foo$&, and foo$$ now become FOO$1, FOO$&, and FOO$$.
scripts/test.sh --runGlob 'vs/{workbench/services/search,workbench/contrib/search,editor/contrib/find}/test/**/*.test.js'
npm run eslint -- src/vs/workbench/services/search/common/replace.ts src/vs/workbench/services/search/test/common/replace.test.ts
npm run precommit -- src/vs/workbench/services/search/common/replace.ts src/vs/workbench/services/search/test/common/replace.test.ts

Refreshed the two changed JavaScript outputs using esbuild with the repository's transpile settings and reused the existing complete build. Full typecheck, full rebuild, and Windows/Linux validation were not rerun for this change.

Copilot AI balanced review requested due to automatic review settings September 5, 2026 10:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Search: case conversion reinterprets literal dollar sequences in captured text

3 participants