Skip to content

fix(sed): preserve escaped replacement characters - #252

Merged
nfischer merged 1 commit into
shelljs:mainfrom
Voyagerroc-Lab:fix/sed-replacement-escaping
Sep 17, 2026
Merged

nfischer merged 1 commit into
shelljs:mainfrom
Voyagerroc-Lab:fix/sed-replacement-escaping

Conversation

@Voyagerroc-Lab

Copy link
Copy Markdown

shx sed silently corrupts escaped replacement characters. For example, replacing foo with the escaped Windows path C:\\temp\\file.txt produces C:.temp.file.txt and exits successfully. This also affects -i, writing the corrupted path to disk.

Unescape replacement characters in one pass, retaining the escaped character instead of substituting a dot. A single pass also prevents adjacent escaped backslashes and slashes from being interpreted twice.

Regression tests exercise file input without -g, stdin with adjacent escapes, and in-place global replacement. They verify output/file contents, exit status, and stderr. All three fail on unchanged main (3d4e06c49a4f4835b9f9aedc728a71d6b7b06493) and pass with the fix.

Validation on Windows, Node v24.19.0:

  • Invoked src/cli.js as a child process with piped input before and after the fix; confirmed both the Windows path and adjacent-backslash/slash reproductions.
  • npm test: 42 passing, 3 existing platform skips for chmod; posttest ESLint passed.
  • npm run check-node-support: passed.
  • git diff --check: passed.

This fixes literal replacement escaping only. It does not implement Unix sed basic regex syntax or numeric backreferences, and does not close the broader compatibility request #225. Other operating systems and Node versions were not run locally.

@nfischer nfischer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! I've triggered the tests to run. If this passes automated tests then I will merge.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (3d4e06c) to head (9daa9ed).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #252   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          126       126           
=========================================
  Hits           126       126           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nfischer
nfischer merged commit 3829f83 into shelljs:main Sep 17, 2026
14 checks passed
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.

2 participants