shelljs / shx

Portable Shell Commands for Node
MIT License
1.72k stars 44 forks source link

fix(sed): better support for converting sed patterns #137

Closed nfischer closed 6 years ago

nfischer commented 6 years ago

This PR does:

  1. Refactors sed-specific pattern-conversion logic. This logic converts a unix-style pattern (e.g., "s/foo/bar/") into the separate parameters expected by shell.sed() (e.g., new RegExp('foo'), 'bar'). This builds the RegExp more carefully to avoid the issue with escaped slashes (as reported in #136).
  2. Explicitly checks for empty patterns and throws an error (previously, these would just silently fail conversion).
  3. Adds tests for the previous two cases.
  4. Refactors the sed tests to be more maintainable by saving file contents in test variables. No change in the logic of already-existing tests.

Fixes #136 Test: 'works with backslashes and forward slashes in pattern' Test: 'does not work with empty regex strings'

codecov-io commented 6 years ago

Codecov Report

Merging #137 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #137   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines          22     40   +18     
=====================================
+ Hits           22     40   +18
Impacted Files Coverage Δ
src/shx.js 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 63bdaa4...e7fb628. Read the comment docs.

nfischer commented 6 years ago

@freitagbr ptal

freitagbr commented 6 years ago

LGTM