Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix escaping in :s substitutions #6891

Merged
merged 3 commits into from
Jul 20, 2021

Conversation

edemaine
Copy link
Contributor

What this PR does / why we need it:
Fix \ escaping in :s substitution commands. The pattern and replacement need to be processed differently, because \s need to be preserved in the pattern for passing to RegExp, whereas they need to actually be processed in the replacement.

  • parsePattern generally leaves backslashes alone, passing them on to the regular expression parser.
  • New parseReplace interprets backslashes. New definitions for \b (backspace), \& (same as $&), and \0 through \9 (same as $1 through $9).
  • Rewrite these parsers in imperative style to avoid any stack overflow.

Which issue(s) this PR fixes
Fixes #6890

Special notes for your reviewer:
I'd like to change how search (/) works as well, for #6520. But I'm finding src/actions/commands/search.ts hard to follow. If you could point me in the right direction, I can try to fix that similarly.

edemaine and others added 3 commits July 19, 2021 16:33
* `parsePattern` generally leaves backslashes alone, passing them on
  to the regular expression parser.
* New `parseReplace` interprets backslashes.  New definitions for `\b`
  (backspace), `\&` (same as `$&`), and `\0` through `\9` (same as
  `$1` through `$9`).
* Rewrite these parsers in imperative style to avoid any stack overflow.

Fixes VSCodeVim#6890
@J-Fields
Copy link
Member

LGTM, thanks!

@J-Fields J-Fields merged commit 10111ef into VSCodeVim:master Jul 20, 2021
@J-Fields
Copy link
Member

I'd like to change how search (/) works as well, for #6520. But I'm finding src/actions/commands/search.ts hard to follow. If you could point me in the right direction, I can try to fix that similarly.

I think the change would be in searchState.ts, particularly set searchString(). Haven't read through this code in a while, so could be wrong, though - let me know if you aren't able to figure it out quickly and I'll dig further to help out.

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.

Backslashes must be duplicated in :s substitution
2 participants