Re: add --color option to md5sum and shaXsum

[Eric Blake]

On 12/24/2013 05:25 AM, Florent Monnier wrote:
> Hello,
> 
> This patch adds a --color option to the commands md5sum and shaXsum.
> The goal is to make it easier to visually identify similarities in a
> list of printed checksums.

Thanks for the patch.  Several comments:

The patch is non-trivial in size; you'll need copyright assignment to
the FSF before we can apply it.  I quit looking at the patch contents
once I saw the size, so as not to taint myself if we end up needing to
reimplement it due to copyright reasons.

> 
> It takes action only if stdout is a tty.

Better would be to copy how other coreutils program use --color, such as
ls.  The option needs to take an optional argument, so that we can do
things like --color=always to force color codes even when not outputting
to a tty (great for testing, also good for pipelines).

You are missing documentation of the new option in coreutils.texi and a
mention of the feature in NEWS.  Adding a new test to the testsuite to
ensure that the option does what you want would also be worthwhile.

Your description did not say WHAT gets colored, and since I didn't read
the body of the patch myself, I'm curious as to more details on what you
are coloring.  This is one case where a screenshot would help your case
(although it may be best if you post the screenshot on an image hosting
service and just post the URL to that location, to minimize list
bandwidth usage - not everyone likes downloading large images).

> 
> This patch can be applied on the version 8.22 of the coreutils.

We prefer patches that are against the latest coreutils.git.  Please
read the HACKING document for more details:
http://git.savannah.gnu.org/cgit/coreutils.git/tree/HACKING

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature