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

Warn when using deprecated rgrep #2657

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

benblank
Copy link

@benblank benblank commented Jan 8, 2023

Just as egrep and fgrep have been deprecated by both the POSIX and GNU versions of grep, Debian has deprecated the rgrep command it injects into the GNU grep package they distribute. (Which Ubuntu inherits, as well.)

The patches applied by Debian now list all three as deprecated (though all three are also listed as "Debian-specific", so perhaps egrep and fgrep have already been removed upstream?), even though it also removes the deprecation warning from egrep and fgrep (rgrep never had one).

This PR adds a new rule, SC2324, which is identical to SC2196 and SC2197 save that it targets rgrep. As a bonus, rgrep has also been added to the list of greps for which -q should be suggested (SC2143).

It probably also makes sense to add all three variants to SC2126 (piping grep to wc -l when grep -c could be used instead), but I didn't discover that until I was typing up this description and my (fairly limited) Haskell skills aren't up to doing it quickly. 😅 If others agree that it should be done though, I'll happily make the change when I have more time in the next few days.

Copy link

@port19x port19x left a comment

Choose a reason for hiding this comment

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

Looks good.

Suggessting -c for egrep, fgrep, rgrep or zgrep would be a bad idea.
Assuming shellcheck compliance, those instances of .{1}grep | wc -l will already be changed to standard grep -c due to the SC2324 you extend here.

@benblank
Copy link
Author

already be changed

I'll have to take your word for it — I'm still working my way though figuring out the code structure. 😉

@port19x
Copy link

port19x commented Jan 16, 2023

It's a two step process. see below:

2023-01-16_07-14

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.

None yet

2 participants