Re: colorize more file types with dircolors ?

[Jim Meyering]

Mike Frysinger <address@hidden> wrote:
> i found this in the archives:
> http://lists.gnu.org/archive/html/bug-coreutils/2004-01/msg00069.html
>
> but no one responded and dircolors hasnt been updated ... Fedora, Mandrake,
> and Gentoo all use similar patches to add support for colorizing more
> things ... all in all, we off support to colorize:
> su: setuid
> sg: setgid
> wo: writeable-other
> wt: same as wo but with sticky bit
>
> ive attached the patch Fedora uses
> -mike
>
> --- fileutils-4.1/src/dircolors.c.orig        Mon Aug  7 19:46:16 2000
> +++ fileutils-4.1/src/dircolors.c     Fri Jul 20 20:56:09 2001

Thanks for reporting that.

Unfortunately, the patch you included (that you said Fedora uses) is
buggy and incomplete.  It should include a change to dircolors.hin that
adds a line per new type, like the lines for ORPHAN, DOOR, etc.

Ideally, it would be accompanied by ChangeLog entries (see below)
and a patch to add a few test cases.
If adding test cases is too hard, then a small, complete
demonstration that the patched program works as documented
would be most welcome.  In this case, it would be commands to
create four files, one of each of the new types, the commands
(dircolors, ls) to make ls display them, and the binary output
of running a command like this (assuming e.g. TERM=xterm):

  ls --color=always ... > LOG_FILE.

...
> @@ -2883,7 +2887,14 @@
>    else
>      {
>        if (S_ISDIR (mode))
> -     type = C_DIR;
> +        {
> +          if ((mode && MODE_WT) == MODE_WT)
> +            type = C_WT;
> +          else if ((mode && MODE_WRO) == MODE_WRO)
> +            type = C_WRO;
> +          else
> +            type = C_DIR;
> +        }

In the code above, the `&&' operators in `mode && MODE_...'
should each be `&'.

The above block should look something like this instead:

        {
          if ((mode & S_IWOTH) && (mode & S_ISVTX))
            type = C_WT;
          else if (mode & S_IWOTH)
            type = C_WRO;
          else
            type = C_DIR;
        }

If there were an added test case (e.g., in tests/ls-2/tests),
this would probably have been caught earlier.

...
> --- fileutils-4.1/src/ls.h.orig       Mon Feb 15 17:27:12 1999
> +++ fileutils-4.1/src/ls.h    Fri Jul 20 21:03:51 2001

There's no need to change ls.h, now.

> +#define MODE_WT 01002
> +#define MODE_WRO 02

Here's what the ChangeLog entries might look like:

        * src/ls.c (indicator_no, indicator_name, color_indicator):
        Add four more: su: setuid, sg: setgid, wo: writeable-other, and
        wt: writeable-other w/sticky bit.
        (print_color_indicator): Set type accordingly.
        * src/dircolors.c: Add four new indicators.
        * src/dircolors.hin: Add four new indicator descriptions.
        Based on a patch by PERSON_NAME.
        <http://include/precise/url...>

Finally, I don't like the names, WRITEOTHERS or WRITEOTHERSSTICKY.
How about OTHER_WRITABLE and STICKY_OTHER_WRITABLE, instead?
(which would mean changing other strings, too)

As you can see, even a small patch to a relatively unimportant
tool (dircolors and the color-highlighting part of ls) is not
trivial to get right.  I hope you understand now, why someone isn't
always able to find the time to review such patches.

Do you feel like finishing the job and submitting a better patch?
If you do, please make the patch relative to coreutils CVS:

  http://savannah.gnu.org/cvs/?group=coreutils