Skip to content

Fix vim replace not escaping $ - #53277

Merged
ConradIrwin merged 2 commits into
zed-industries:mainfrom
phdavis1027:fix-vim-dollar-sign-not-escaped-in-replace
Apr 27, 2026
Merged

Fix vim replace not escaping $#53277
ConradIrwin merged 2 commits into
zed-industries:mainfrom
phdavis1027:fix-vim-dollar-sign-not-escaped-in-replace

Conversation

@phdavis1027

@phdavis1027 phdavis1027 commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Self-Review Checklist:

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Closes ##42292

The text inserted in the search ('$SEARCH') and replace ('$$OTHER') inputs of the top-panel is a little anti-aesthetic, but that seems out of scope for this issue.

Release Notes:

  • '$' in the second clause of vim-style '%s/find/replace/g' actions is correctly escaped.

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Apr 6, 2026
@zed-community-bot zed-community-bot Bot added the first contribution the author's first pull request to Zed. NOTE: the label application is automated via github actions label Apr 6, 2026
@phdavis1027
phdavis1027 force-pushed the fix-vim-dollar-sign-not-escaped-in-replace branch 2 times, most recently from 11b9e1a to bdb9eaa Compare April 7, 2026 01:01
@phdavis1027
phdavis1027 marked this pull request as ready for review April 7, 2026 01:02
@zed-codeowner-coordinator
zed-codeowner-coordinator Bot requested review from a team, as-cii and dinocosta and removed request for a team April 7, 2026 01:02
@phdavis1027
phdavis1027 force-pushed the fix-vim-dollar-sign-not-escaped-in-replace branch from bdb9eaa to 71924a3 Compare April 7, 2026 01:03
@maxdeviant maxdeviant changed the title fix: vim replace doesn't escape $, expands to "" Fix vim replace not escaping $ Apr 7, 2026
@phdavis1027
phdavis1027 marked this pull request as draft April 10, 2026 00:52
@phdavis1027

phdavis1027 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

This passed my initial desk-check, but after sitting down to make some improvements it no longer seems to be behaving. I pulled down the latest from main, but I couldn't say for sure that that's what broke it. Marking as draft until I can make it work again.

EDIT: Re-opening because it turn out I was test from the Zed I was writing Zed in (preview), not the Zed I was writing. Oops! Fix still works as intended.

I looked into making the text populated by the search bar a little more aesthetic, but the existing tree-sitter-regex crate does not have anything concerned with the replacement clause. Alternatively, we could parse and manually apply highlights. regex_automata etc have a bunch of utilities which almost parse, but nothing that quite works as far as I can see. The "grammar" of the replacement clause is pretty straightforward, so manual parsing is an option, but it felt a little smelly replicating regex crate internals.

Altogether, since the replacement bar is not highlighted at all in today's Zed, I leave the design choice of how/if to do that for another commit, modulo other opinions.

@phdavis1027
phdavis1027 marked this pull request as ready for review April 10, 2026 14:33
@zed-codeowner-coordinator
zed-codeowner-coordinator Bot requested a review from a team April 10, 2026 14:34
@SomeoneToIgnore
SomeoneToIgnore removed the request for review from a team April 11, 2026 16:51
@ConradIrwin
ConradIrwin added this pull request to the merge queue Apr 27, 2026
@ConradIrwin

Copy link
Copy Markdown
Member

Thanks for this!

@ConradIrwin

Copy link
Copy Markdown
Member

@zed-zippy approved

Merged via the queue into zed-industries:main with commit 663bc9d Apr 27, 2026
31 checks passed
ebaah46 pushed a commit to ebaah46/zed that referenced this pull request May 6, 2026
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #zed-industries#42292

The text inserted in the search ('\$SEARCH') and replace ('$$OTHER')
inputs of the top-panel is a little anti-aesthetic, but that seems out
of scope for this issue.

Release Notes:

- '$' in the second clause of vim-style '%s/find/replace/g' actions is
correctly escaped.

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
kathbigra pushed a commit to kathbigra/zed that referenced this pull request May 10, 2026
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #zed-industries#42292

The text inserted in the search ('\$SEARCH') and replace ('$$OTHER')
inputs of the top-panel is a little anti-aesthetic, but that seems out
of scope for this issue.

Release Notes:

- '$' in the second clause of vim-style '%s/find/replace/g' actions is
correctly escaped.

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #zed-industries#42292

The text inserted in the search ('\$SEARCH') and replace ('$$OTHER')
inputs of the top-panel is a little anti-aesthetic, but that seems out
of scope for this issue.

Release Notes:

- '$' in the second clause of vim-style '%s/find/replace/g' actions is
correctly escaped.

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #zed-industries#42292

The text inserted in the search ('\$SEARCH') and replace ('$$OTHER')
inputs of the top-panel is a little anti-aesthetic, but that seems out
of scope for this issue.

Release Notes:

- '$' in the second clause of vim-style '%s/find/replace/g' actions is
correctly escaped.

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement first contribution the author's first pull request to Zed. NOTE: the label application is automated via github actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants