Page MenuHomeFreeBSD

GNU grep: use --color=auto by default
Needs RevisionPublic

Authored by arrowd on Apr 12 2019, 8:28 AM.

Details

Summary

GNU grep defaults to --color=auto when --color is passed, but if no flag is given, no coloring applied.
Recent GNU grep in Ubuntu colorises its output by default, so we are following the behaviour there.

Test Plan

grep foo file_with_foo now highlights found substrings.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 23642
Build 22624: arc lint + arc unit

Event Timeline

arrowd created this revision.Apr 12 2019, 8:28 AM
avg resigned from this revision.Apr 25 2019, 8:07 PM
0mp requested changes to this revision.May 6 2019, 7:47 AM
0mp added a subscriber: 0mp.
0mp added inline comments.
gnu/usr.bin/grep/grep.c
1721

You've got style(9) issues.

This revision now requires changes to proceed.May 6 2019, 7:47 AM
0mp added inline comments.May 6 2019, 7:53 AM
gnu/usr.bin/grep/grep.c
1721

Actually, this is not really style(9)-compliant file as I look at it now...

Why this if statement had to be moved to a different place?

arrowd added a comment.May 6 2019, 8:19 AM

That if(color_option == 2) block checks terminal features and enables/disables output coloring based on that.

The problem is that this check is currently located inside command-line handling loop while ((opt = get_nondigit_option (argc, argv, &default_context)) != -1). Thus, it only gets executed if you actually pass --color=something. So, I just moved it out of the loop, so it would be always be executed.

0mp added a comment.May 8 2019, 9:20 PM

LGTM.

It would be great to update the manual page at the same time.

In D19888#435371, @0mp wrote:

LGTM.

I don't have src commit bit.

It would be great to update the manual page at the same time.

What should be updated, exactly? The --color option is already documented.

0mp added a comment.May 13 2019, 2:18 PM
In D19888#435371, @0mp wrote:

It would be great to update the manual page at the same time.

What should be updated, exactly? The --color option is already documented.

The default behavior of --color flag. I am not sure if that's necessary at this point.

Also, I forgot that we are talking about the GNU grep here.

Then your D20253 was perfectly sensible, why did you discarded it?

What needs to be done to get this in?