Bug 83063 - refactoring patch for colorls.[c]sh that makes more sense
refactoring patch for colorls.[c]sh that makes more sense
Status: CLOSED CANTFIX
Product: Red Hat Raw Hide
Classification: Retired
Component: coreutils (Show other bugs)
1.0
All Linux
low Severity medium
: ---
: ---
Assigned To: Tim Waugh
: FutureFeature
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2003-01-29 17:01 EST by Scott R. Godin
Modified: 2007-03-27 00:00 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-10-18 15:32:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch for /etc/profile.d/colorls.[c]sh (605 bytes, patch)
2003-01-29 17:02 EST, Scott R. Godin
no flags Details | Diff
patch file for colorls.csh (647 bytes, patch)
2003-01-29 17:26 EST, Scott R. Godin
no flags Details | Diff
patch for colorls.sh (776 bytes, patch)
2003-01-29 17:27 EST, Scott R. Godin
no flags Details | Diff

  None (edit)
Description Scott R. Godin 2003-01-29 17:01:38 EST
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
Comment 1 Scott R. Godin 2003-01-29 17:02:25 EST
Created attachment 89696 [details]
patch for /etc/profile.d/colorls.[c]sh
Comment 2 Scott R. Godin 2003-01-29 17:26:39 EST
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 :)
Comment 3 Scott R. Godin 2003-01-29 17:27:09 EST
Created attachment 89698 [details]
patch for colorls.sh
Comment 4 Tim Waugh 2003-05-28 04:52:57 EDT
The patch is incorrect: there is no duplication.  Each line in colorls.sh is
different.
Comment 5 Scott R. Godin 2003-05-28 10:55:28 EDT
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.
Comment 6 Scott R. Godin 2003-05-28 11:12:49 EDT
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. 

Comment 7 Bill Nottingham 2006-08-07 21:49:57 EDT
'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.
Comment 8 Bill Nottingham 2006-10-18 15:32:55 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.