Warn when using deprecated rgrep #2657
Open
+6
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
port19x
approved these changes
Jan 13, 2023
port19x
left a comment
port19x
left a comment
There was a problem hiding this comment.
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.
Author
I'll have to take your word for it — I'm still working my way though figuring out the code structure. 😉 |
|
It's a two step process. see below:
|
koalaman
force-pushed
the
master
branch
4 times, most recently
from
50074dc to
eac8eff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just as
egrepandfgrephave been deprecated by both the POSIX and GNU versions of grep, Debian has deprecated thergrepcommand 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
egrepandfgrephave already been removed upstream?), even though it also removes the deprecation warning fromegrepandfgrep(rgrepnever had one).This PR adds a new rule, SC2324, which is identical to SC2196 and SC2197 save that it targets
rgrep. As a bonus,rgrephas also been added to the list of greps for which-qshould be suggested (SC2143).It probably also makes sense to add all three variants to SC2126 (piping grep to
wc -lwhengrep -ccould 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.