Re: Not sure how to best reply re: dir_colors situation

[Kamil Dudka]

On Wednesday 10 of June 2009 19:12:18 Pádraig Brady wrote:
> Kamil Dudka wrote:
> > Hi Pádraig,
> >
> > On Sunday 07 June 2009 16:35:23 Pádraig Brady wrote:
> >> Turn off completely is my vote, as hardlinks have their own column.
> >> I polled my local LUG with a non leading question and
> >> nobody came up with a reason for coloring hardlinks.
> >>
> >> Eric voted for "not enable by default".
> >>
> >> Jim checked the functionality in originally.
> >>
> >> So I guess the middle ground is best of "not enable by default",
> >> though that will require adding documentation for the option.
> >> That's probably best to add in the dir_colors man page which
> >> is part of the linux man pages collection.
> >> Hmm, would this page be better located in coreutils?
> >
> > enclosed is the patch to disable it by default. The patch also renames
> > HARDLINK to MULTIHARDLINK and 'hl' to 'mh' (as Joshua proposed and Jim
> > acked). It does not import the dir_colors man page yet.
>
> Great!
>
> So you've decided to not support existing dir_colors files with HARDLINK in
> them. I think this is fine as I expect people who used this feature to be
> _very_ few. In fact doing that is probably a good thing as if users had
> created and modified a .dir_colors file using `dircolors --print-database`
> as a template, they will now be given this error:
>   dircolors: `~/.dir_colors':68: unrecognized keyword HARDLINK
> That means they can just delete that line and get back to non coloured
> hardlinks which they probably wanted in any case.
>
> more comments below...
>
> diff --git a/NEWS b/NEWS
> index 29b09a0..d6d88c8 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,13 @@ GNU coreutils NEWS                                    -*-
> outline -*- truncate -s failed to skip all whitespace in the option
> argument in some locales.
>
> +** Changes in behavior
> +
> +  ls --color: hard link highlighting is now disabled by default, it can be
> +  enabled by changing the LS_COLORS environment variable. You can do it
> +  explicitly in your profile. Note the init string was renamed from
> HARDLINK +  to MULTIHARDLINK and the appropriate color code from 'hl' to
> 'mh'.
>
> How about:
>
> ls --color: files with multiple hard links are no longer colored
> differently by default. That can be enabled by changing the LS_COLORS
> environment variable. You can control that using the MULTIHARDLINK
> dircolors input variable which corresponds to the 'mh' LS_COLORS item. Note
> these variables were renamed from 'HARDLINK' and 'hl' which were available
> since
> coreutils-7.1 when this feature was introduced.
>
> diff --git a/tests/ls/multihardlink b/tests/ls/multihardlink
> new file mode 100755
> index 0000000..0d626d6
> --- /dev/null
> +++ b/tests/ls/multihardlink
> +
> +# regular file - not hard linked
> +LS_COLORS="mh=$code_mh" ls -U1 --color=always file > out || fail=1
> +printf "file\n" > out_ok || fail=1
>
> Probably in this case should || framework_failure
>
> +compare out out_ok || fail=1
> +
> +# hard links
> +LS_COLORS="mh=$code_mh" ls -U1 --color=always file1 file2 > out || fail=1
> +printf "\033[0m\033[44;37mfile1\033[0m
> +\033[44;37mfile2\033[0m
> +\033[m" > out_ok || fail=1
>
> || framework_failure
>
> +compare out out_ok || fail=1
> +
> +# hard links and png
>
>  # hard links and png (hardlink coloring takes precedence)
>
> +mv file2 file2.png || framework_failure
> +LS_COLORS="mh=$code_mh:*.png=$code_png" ls -U1 --color=always file1
> file2.png > out || fail=1
>
> line is too long. You could split like:
> LS_COLORS="mh=$code_mh:*.png=$code_png" \
> ls -U1 --color=always file1 file2.png > out || fail=1
>
> +printf "\033[0m\033[44;37mfile1\033[0m
> +\033[44;37mfile2.png\033[0m
> +\033[m" > out_ok || fail=1
>
> || framework_failure
>
> +compare out out_ok || fail=1
>
> +# hard links and png (hard links higmhighting disabled)
>
>  # hard links and png (hardlink coloring disabled => png coloring enabled)
>  I'd also check or at least comment that mh=00 and mh not present give the
> same results
>
> +LS_COLORS="mh=:*.png=$code_png" ls -U1 --color=always file1 file2.png >
> out || fail=1 +printf "file1
> +\033[0m\033[01;35mfile2.png\033[0m
> +\033[m" > out_ok || fail=1
> +compare out out_ok || fail=1
> +
> +Exit $fail

Thanks for review! I've put it together.

Kamil

0001-ls-color-do-not-colorize-files-with-multiple-hard.patch
Description: Text Data