Red Hat Bugzilla – Bug 83063
refactoring patch for colorls.[c]sh that makes more sense
Last modified: 2007-03-27 00:00:26 EDT
From Bugzilla Helper: User-Agent: Mozilla/5.0 Galeon/1.2.6 (X11; Linux i686; U;) Gecko/20020830 Description of problem: basically I took out the redundant code in the elsif, and moved the things that don't change to after that point. (this is the refactoring part) also added in aliases I use frequently enough (damn near daily) to merit being enabled system-wide, and meriting an RFE (at least in my humble opinion) So, this is in essence, two patches. In addition to ls, ll, and l. we have: ls changed to include -F switch so that all files and directories have the * / @ = modifiers placed after each to give a better visual feedback to the new user (in ADDITION to color) since color alone is not enough to distinguish whether the file "zone.info.com" is executable or not. (DIR_COLORS specifies that .com is a DOS executable and colorizes it like one, whether or not the executable bit is set. DOH!) la -- like ls but with dotfiles lf -- like ll but without dotfiles ll -- changed to include dotfiles lr -- list directory path recursively to show subfolders but with the long-form -l to show perms too. lsd -- used for listing a directory path *without* recursing into it to view perms on it easily. (originally I'd used ld instead of lsd, before I realized that it already existed in /usr/bin/ld ! oops!) and of course, lp was already taken. so much for mnemonic aliases. :-) hopefully enough people find these useful to make their inclusion worthwhile, but even without them, the refactoring makes sense. that elsif was just too redundant. --scott Version-Release number of selected component (if applicable): How reproducible: Always Steps to Reproduce: Additional info: patch files attached for /etc/profile.d/colorls.csh and colorls.sh
Created attachment 89696 [details] patch for /etc/profile.d/colorls.[c]sh
Created attachment 89697 [details] patch file for colorls.csh ignore previous patch (submitted as .tgz and the file won't display properly here and it won't let me delete the thing :)
Created attachment 89698 [details] patch for colorls.sh
The patch is incorrect: there is no duplication. Each line in colorls.sh is different.
it's a *refactoring* patch *OBVIOUSLY* each line is different. That's the POINT of the patch! :-) There's no need to add - 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 within the elsif clauses when merely + alias ls='ls -F --color=tty' 2>/dev/null will do the trick for ALL of the above, AND having these below processed OUTside the elseif loop (and again these additional aliases are more concise). +alias ll='ls -la ' +alias l.='ls -ad .?* ' +alias la='ls -a ' +alias lf='ls -l ' This is what refactoring is all about. Code _cleanup_. In other words, it's easier to read, takes slightly less space, and does the job better. What's not to like? let me be more succinct. The duplication is *within* the aliases themselves as well. Each alias does not need --color=tty within it; only the ls alias itself does. The other aliases will alias ls with switches. Therefore they are not part of the elseif loop and should be placed outside. *Additionally*, 'correction' to the aliases to 'make more sense' has been performed with regards to ll la lf and l. I'm amenable to being contacted via e-mail if there are any other questions regarding these two patches. Again, we aren't fixing a bug here, we're refactoring code to improve on the original.
Allow me to add one further comment. +alias lsd='ls -ld ' +alias lr='ls -lR ' These two can both be removed from the above patch without affecting the intent of the patch, and are merely suggested additions. If they are unwelcome additions, fine, don't include them. I don't wish to force them on anyone, although nothing is stopping anyone from simply not using them -- their presence would not affect anything else one whit.
'Red Hat Raw Hide' refers to the development tree for Red Hat Linux. Red Hat Linux is no longer supported by Red Hat, Inc. If you are still running Red Hat Linux, you are strongly advised to upgrade to a current Fedora Core release or Red Hat Enterprise Linux or comparable. Some information on which option may be right for you is available at http://www.redhat.com/rhel/migrate/redhatlinux/. Red Hat apologizes that these issues were not resolved in a more timely manner. However, we do want to make sure that important don't slip through the cracks. If these issues are still present in a current release, such as Fedora Core 5, please move these bugs to that product and version. Note that any remaining Red Hat Raw Hide bugs will be closed as 'CANTFIX' on September 30, 2006. Thanks again for your help.
Red Hat Linux is no longer supported by Red Hat, Inc. If you are still running Red Hat Linux, you are strongly advised to upgrade to a current Fedora Core release or Red Hat Enterprise Linux or comparable. Some information on which option may be right for you is available at http://www.redhat.com/rhel/migrate/redhatlinux/. Closing as CANTFIX.