|Summary:||[PATCH] Enable ls colors when COLORTERM is defined|
|Product:||Base System||Reporter:||D Green <dfrg>|
|Component:||bin||Assignee:||Kyle Evans <kevans>|
|Severity:||Affects Some People||CC:||cem, kevans, yuripv|
Description D Green 2018-07-28 09:51:07 UTC
Created attachment 195523 [details] Enable colors when COLORTERM defined Color output is currently supported for ls via the CLICOLOR environment variable which is unique to both the FreeBSD project and that particular binary. The defacto standard is COLORTERM and is set by all terminal emulators derived from rxvt or those based on vte (the majority). Attached patch enables color output for interactive shells started within the above terminal emulators. All others, in particular system console and non-interactive shells, remain unchanged. It's my first patch so apologies if the format isn't exactly as required.
Comment 1 Conrad Meyer 2018-07-28 20:05:19 UTC
Comment 2 Yuri Pankov 2018-07-28 20:14:32 UTC
I think there's a difference between CLICOLOR and COLORTERM in that the former *needs* to be set by the user, and the latter is likely to be set by the software (ie, terminal emulator) itself, so this change will mean that we are forcing colored ls output on those whose terminal emulator did set COLORTERM on its own.
Comment 3 Conrad Meyer 2018-07-28 20:20:19 UTC
(In reply to Yuri Pankov from comment #2) I guess there is that distinction, yes. Perhaps it is possible for users to configure their terminal to disable setting the color variable, or explicitly unset it in .shellrc. Nevertheless, it doesn't really make sense for ls to have its own variable that nothing else uses.
Comment 4 D Green 2018-07-28 20:47:42 UTC
(In reply to Yuri Pankov from comment #2) Yes, colors would be automatic for any terminal setting COLORTERM wheres CLICOLOR requires explicit configuration. The existing check that TERM is suitable is unchanged. I can't think of a non-contrived reason for a user of a color terminal not wanting that behavior but if so then unsetting COLORTERM via .shellrc is trivial and already necessary as many other binaries external to the project enable colors automatically when it is defined. As with Conrad, I think it's better to follow the defacto standard rather than use a unique variable (or two if you count CLICOLOR_FORCE). Currently you either know to set the non-portable CLICOLOR or forgo color functionality.
Comment 5 commit-hook 2018-08-08 21:51:53 UTC
A commit references this bug: Author: kevans Date: Wed Aug 8 21:51:20 UTC 2018 New revision: 337506 URL: https://svnweb.freebsd.org/changeset/base/337506 Log: ls(1): Enable colors with COLORTERM is set in the environment COLORTERM is the de facto standard, while CLICOLOR is generally specific to FreeBSD and ls(1). PR: 230101 Submitted by: D Green <email@example.com> (with manpage additions by myself) Reviewed by: cem ("LGTM" in PR; pre-manpage changes) MFC after: 1 week Changes: head/bin/ls/ls.1 head/bin/ls/ls.c
Comment 6 Kyle Evans 2018-08-08 21:55:06 UTC
Comment 7 commit-hook 2018-08-15 01:29:36 UTC
A commit references this bug: Author: kevans Date: Wed Aug 15 01:29:02 UTC 2018 New revision: 337826 URL: https://svnweb.freebsd.org/changeset/base/337826 Log: MFC r337506: ls(1): Enable colors with COLORTERM is set in the environment COLORTERM is the de facto standard, while CLICOLOR is generally specific to FreeBSD and ls(1). PR: 230101 Changes: _U stable/11/ stable/11/bin/ls/ls.1 stable/11/bin/ls/ls.c