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
