Skip to content

Add -B, -A, and -C options to grep #1206

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

Merged
merged 21 commits into from
Apr 19, 2025
Merged

Add -B, -A, and -C options to grep #1206

merged 21 commits into from
Apr 19, 2025

Conversation

abluescarab
Copy link
Contributor

@abluescarab abluescarab commented Apr 4, 2025

Adds the -B (before context) and -A (after context) options to grep. I may have gone overboard on tests but I wanted to make sure that most feasible configurations work (combined, separate, with space, without space).

Notes

  • Modifies globStart to accept a function with a parameter args. This allows -B and -A to be specified separately and a function to determine where the post-regex arguments begin.
  • Enforces these styles:
    • .editorconfig: Single quotes in JavaScript files
    • .eslintrc.json: radix option to off (don't require base for parseInt())
    • package.json: Prettier arrowParens option to avoid (don't wrap single parameters in parentheses for arrow functions)

Usage

grep -B <num> [args...]
grep -A <num> [args...]

Examples

# show 3 lines before
grep -B3 string file1 file2 ...
grep -B 3 string file1 file2 ...

# show 2 lines after
grep -A2 string file1 file2 ...
grep -A 2 string file1 file2 ...

# show 3 lines before and 2 after
grep -B3A2 string file1 file2 ...
grep -B3 -A2 string file1 file2 ...

# show 3 lines before and after
grep -AB3 string file1 file2 ...
grep -AB 3 string file1 file2 ...

Copy link
Member

@nfischer nfischer left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this feature. I've provided some initial feedback that you can get started on now, but I'll need to spend more time reviewing this (expect more feedback later).

Copy link

codecov bot commented Apr 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.46%. Comparing base (75d26de) to head (1fc99f6).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1206      +/-   ##
==========================================
+ Coverage   97.37%   97.46%   +0.08%     
==========================================
  Files          35       35              
  Lines        1410     1459      +49     
==========================================
+ Hits         1373     1422      +49     
  Misses         37       37              

☔ View full report in Codecov by Sentry.
📢 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.

@abluescarab
Copy link
Contributor Author

Sorry about the commit mess. I'm not sure why checks are failing on macos and ubuntu now.

@abluescarab abluescarab requested a review from nfischer April 5, 2025 06:13
@nfischer
Copy link
Member

nfischer commented Apr 6, 2025

Sorry about the commit mess. I'm not sure why checks are failing on macos and ubuntu now.

Oh, this should be a simple fix. Can you run npm run gendocs in a terminal? This will regenerate README.md, then you can commit the README changes. That's probably the issue.

If that doesn't fix it, then try running the tests locally and run git status. It looks like there's some uncommitted file after the tests finish, and git status should tell you exactly what file that is.

@abluescarab
Copy link
Contributor Author

Oh, this should be a simple fix. Can you run npm run gendocs in a terminal? This will regenerate README.md, then you can commit the README changes. That's probably the issue.

Hey, that worked, thanks! Hopefully everything looks good now.

@nfischer nfischer added feature bash compat Compatibility issues with bash or POSIX behavior labels Apr 6, 2025
@abluescarab abluescarab requested a review from nfischer April 8, 2025 00:15
@nfischer nfischer added this to the v0.10.0 milestone Apr 10, 2025
Copy link
Member

@nfischer nfischer left a comment

Choose a reason for hiding this comment

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

LGTM, but with a few more suggestions.

Thanks for working on this feature!

@nfischer nfischer merged commit bfd06fa into shelljs:main Apr 19, 2025
11 checks passed
@nfischer nfischer changed the title Add -B and -A options to grep Add -B, -A, and -C options to grep Apr 19, 2025
@nfischer
Copy link
Member

nfischer commented May 9, 2025

Released in shelljs v0.10. Thanks for the contribution!

nfischer added a commit to shelljs/shx that referenced this pull request May 9, 2025
This adds support for shelljs/shelljs#1206. This
requires special parsing logic on the shx side.

Test: npm test
Test: ./lib/cli.js grep -A 1 'foo' ~/foobartest.txt
Test: cat ~/foobartest.txt | ./lib/cli.js grep -A 1 'foo'
nfischer added a commit to shelljs/shx that referenced this pull request May 9, 2025
This adds support for shelljs/shelljs#1206. This
requires special parsing logic on the shx side.

Test: npm test
Test: ./lib/cli.js grep -A 1 'foo' ~/foobartest.txt
Test: cat ~/foobartest.txt | ./lib/cli.js grep -A 1 'foo'
nfischer added a commit to shelljs/shx that referenced this pull request May 10, 2025
This adds support for shelljs/shelljs#1206. This
requires special parsing logic on the shx side.

Test: npm test
Test: ./lib/cli.js grep -A 1 'foo' ~/foobartest.txt
Test: cat ~/foobartest.txt | ./lib/cli.js grep -A 1 'foo'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash compat Compatibility issues with bash or POSIX behavior feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants