Bug 430827
Summary: | shouldn't override user's LS_COLORS | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mikel Ward <mikel> | ||||
Component: | coreutils | Assignee: | Ondrej Vasik <ovasik> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | low | Docs Contact: | |||||
Priority: | low | ||||||
Version: | 8 | CC: | twaugh | ||||
Target Milestone: | --- | Keywords: | Reopened | ||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | coreutils-6.9-16.fc8 | Doc Type: | Bug Fix | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2008-03-17 12:23:59 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Mikel Ward
2008-01-29 23:37:31 UTC
Thanks for suggestion... Actually ... it could cause ls working without colors when no ls aliases would be defined. Therefore used if [ -n "$LS_COLORS" ]; then #do not override user LS_COLORS but use them alias ll='ls -l --color=tty' 2>/dev/null alias l.='ls -d .* --color=tty' 2>/dev/null alias ls='ls --color=tty' 2>/dev/null exit fi With following change built as coreutils-6.10-4.fc9 in rawhide. Will be in next update of F8. That check doesn't do anything useful. The test needs to be around the dircolors stuff. If you want to keep the ls aliases, then do if [ -z "$LS_COLORS" ]; then [ -f ~/.dircolors.$TERM ] && eval `dircolors ~/.dircolors.$TERM ... fi Why do you think that this check doesn't do anything useful? If not zero length of LS_COLORS then expect LS_COLORS correctly set and use colored aliases for ls ... otherwise make the rest of script as it was before... It works on my machine without problems - where you see the problem? Your LS_COLORS will be not overwritten by colorls.sh and you will have colored ls as you have specified in LS_COLORS... The problem is these lines: COLORS=/etc/DIR_COLORS [ -e "/etc/DIR_COLORS.$TERM" ] && COLORS="/etc/DIR_COLORS.$TERM" [ -e "$HOME/.dircolors" ] && COLORS="$HOME/.dircolors" [ -e "$HOME/.dircolors.$TERM" ] && COLORS="$HOME/.dircolors.$TERM" [ -e "$HOME/.dir_colors" ] && COLORS="$HOME/.dir_colors" [ -e "$HOME/.dir_colors.$TERM" ] && COLORS="$HOME/.dir_colors.$TERM" [ -e "$COLORS" ] || return They override my LS_COLORS if I have already set it. Actually it's this line: eval `dircolors --sh "$COLORS"` but I'd wrap all that in the LS_COLORS test. Created attachment 293693 [details] colorls.sh which doesn't override user defined LS_COLORS I know what overrides LS_COLORS - and current version in RAWHIDE DOES NOT override LS_COLORS ... I think the best way is to attach the shell script... I think you don't understand where I added lines from comment #1. It is more clear now? I know that there is now doubled code, but anyway I have to rework colorls.sh (which is quiet old - as were DIR_COLORS shipped until coreutils 6.10) because of 256 color support, so I will try to improve it somehow. You don't understand. Try running dircolors ~/.dircolor. You will see it prints commands to set LS_COLORS. Running eval `dircolors ~/.dircolors` runs the commands printed by dircolors ~/.dircolors. It is very clear that dircolors is setting LS_COLORS. My environment sets LS_COLORS before /etc/profile.d/colorls.sh is run, so my custom LS_COLORS setting is lost. Ok, so maybe I don't understand... so please try to explain WHERE I'm using eval `dircolors ~/.dircolors` in RAWHIDE version of colorls.sh in the case that you have your LS_COLORS set. I'm sorry for typo in attachment(this version has -z instead of -n), but in comment #1 is correct sequence used in .sh script shipped in RAWHIDE branch(coreutils-6.10-4.fc9) and attachment was here only for clarification where the code from comment #1 is used. First lines of the RAWHIDE colorls.sh (with additional comments for COLOR_LS value set): # color-ls initialization if [ -n "$LS_COLORS" ]; then # if your LS_COLORS variable has nonzero length # do not override user defined colors and use them #set aliases for colored ls alias ll='ls -l --color=tty' 2>/dev/null alias l.='ls -d .* --color=tty' 2>/dev/null alias ls='ls --color=tty' 2>/dev/null #THIS WILL EXIT FROM THE SHELL SCRIPT return fi # you will never get there when you had LS_COLORS set from e.g. ~/.bashrc # (because of non zero length "if" with return) ... rest of the shell script which will get LS_COLORS from e.g. /etc/DIR_COLORS Could you please clarify where you see the line which will override your LS_COLORS? Ah, sorry, I missed the exit in comment 1 (and the corresponding return in the new version). To me the following layout would make more sense: if [ -z "$LS_COLORS" ]; then COLORS=... eval `dircolors --sh "$COLORS"` fi if [ -n "$LS_COLORS" ]; then alias ll=... fi As I said in comment #6 , colorls.sh(.csh) has to be rewritten because of 256 color support and because of broken design of DIR_COLORS.xterm usage ... this file was created for light background (default in XTERM), but TERM type XTERM is used by many others terminals(e.g. gnome terminal with default black background). I will see if your layout from comment #9 will be usable in reworked colorls.sh, but definitely is better than the old one. Thanks for suggestion... coreutils-6.9-13.fc8 has been pushed to the Fedora 8 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update coreutils'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-1844 coreutils-6.9-13.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report. Sorry, I have to reopen the bug because of the complaints about the current behaviour. The old one .sh/.csh script overrides user defined LS_COLORS, but makes LS_COLORS flexible - you may have basic LS_COLORS from /etc/DIR_COLORS and once you will start xterm, they will get overriden by /etc/DIR_COLORS.xterm . This is broken by the new version . So I have to remove those changes. What do you think about the possibility of env.variable like USER_LS_COLORS - which will block overwriting of LS_COLORS? This is more suitable solution. Let me know about your opinion - but I can't use current approach with [ -z LS_COLORS ];. So your use case is: 1. /etc/DIR_COLORS is sourced when you log in to X (where TERM is empty) 2. you start an xterm and want /etc/DIR_COLORS.xterm to be run One thought is to only run dircolors (and hence set LS_COLORS) if TERM is set. This could be done by wrapping the whole thing in an "if [ -n "$TERM" ]; then" or similar. No, other user case is: 1) runlevel 3 (TERM is linux(not empty), uses /etc/DIR_COLORS ) 2) startx (and then start xterm) - TERM is xterm, should use /etc/DIR_COLORS.xterm but the behaviour is basically same like you described... Previous solution with [ -n $LS_COLORS ] is wrong and I don't see any suitable solution - as you should be able to switch between /etc/DIR_COLORS.xterm, /etc/DIR_COLORS (and in devel branch additionally with /etc/DIR_COLORS.256colors if the term is able to support 256 colors). So rely only on the TERM variable is not usable idea - basically the same as the [ -n $LS_COLORS ] . Solution via additional env.variable will work in all cases - but if used, should be documented somehow ( I used that approach in latest builds, to make old behaviour running again ). At any case - you could always add export LS_COLORS=<your user settings> to the .bashrc , which is executed after shell scripts from /etc/profile.d/ (like colorls.sh) - and override settings from colorls shell script. I like the whole idea of keeping user defined LS_COLORS variable, but I don't see any easy solution without additional env.variable ("if [ -z USER_LS_COLORS ]; then <override LS_COLORS script> fi" sounds ok for me - it makes you able to keep LS_COLORS if you wish) . Testing whether $TERM is set would work in those situations: 1) TERM is set, LS_COLORS is unset => run dircolors to set LS_COLORS 2) I think startx starts bash as a login shell, which would erase the previous environment, and thus /etc/profile.d/colorls.sh would be run for the first time when the xterm is started, but I could be wrong (I don't have a Linux box in front of me to test) The only other case to consider is if you start a terminal from inside another one, e.g. run gnome-terminal inside an xterm. What I find most problematic is that it runs from /etc/profile as well as /etc/bashrc. I will get more evidence and then elaborate. OK. I was relying on .zshenv not getting clobbered by /etc/zprofile and /etc/zshrc. I've now got rid of .zshenv and made my zsh setup more like bash, and I can deal with the current setup. Thanks for your help. Ok, closing once more time CURRENTRELEASE as there is now a way to keep user defined LS_COLORS via env. variable - which should be harmless for the rest unless is set. |