Conversation
includes the i flag to ignored upper/lower case differences
nfischer
left a comment
There was a problem hiding this comment.
This change looks good so far, but please also:
npm run gendocs- Add a simple test in
test/grep.js
| //@ + `-v`: Invert `regex_filter` (only print non-matching lines). | ||
| //@ + `-l`: Print only filenames of matching files | ||
| //@ + `-l`: Print only filenames of matching files. | ||
| //@ + `-i`: Ignored upper/lower case differences. |
There was a problem hiding this comment.
nit: can you change this sentence to simply "Ignore case."?
nfischer
left a comment
There was a problem hiding this comment.
One comment in-line, otherwise LGTM
| @@ -0,0 +1 @@ | |||
| Test3 | |||
There was a problem hiding this comment.
This is OK, but could you move these files under test/resources/grep/case1*? That should fix the other failures.
There was a problem hiding this comment.
OK, I moved it, and sorry for my thoughtless actions.
|
|
||
| var contents = file === '-' ? pipe : fs.readFileSync(file, 'utf8'); | ||
| if (options.ignoreCase) { | ||
| regex = new RegExp(regex, 'i'); |
There was a problem hiding this comment.
Sorry, one more fix: move the ignoreCase logic out of the for-loop. The CI failure is because we're repeatedly re-assigning regex, but we should only assign to it once.
There was a problem hiding this comment.
Oh, it's a low-level error, i fixed it, sorry.
Codecov Report
@@ Coverage Diff @@
## master #876 +/- ##
==========================================
+ Coverage 95.8% 95.81% +<.01%
==========================================
Files 34 34
Lines 1264 1266 +2
==========================================
+ Hits 1211 1213 +2
Misses 53 53
Continue to review full report at Codecov.
|
|
@ppsleep thanks for the great contribution! |
grep includes the i flag to ignored upper/lower case differences