Hi Mara, On 2013-04-03 17:08, Mara Kim wrote: > I thought you guys might enjoy this simple tool I wrote. It's under GPL so > use it, hack it, fork it, ignore it, etc.
I'm sorry that the first reply has to be criticism, but since you posted it on a
mailing list, I guess you're looking for feedback.
First of all, please stop using variables to contain things like "echo", or
whatever. The prevalence of $TO_X all over the script is horrific and should be
completely avoided. Almost always this is because people want to hardcode the
path, which should just be done by setting $PATH at the top of the script, but
it seems you're not doing that, so I have absolutely no idea what you hoped to
achieve by littering your script with that.
You also have a lot of mixing of bash and POSIX idioms. The result is that it's
both not POSIX portable, and fails to use bash features when it could (QED,
function vs. [).
Your script will break on paths or names with pipes in them, or newlines.
There are a few other problems, but I don't think it's constructive for me to
nag about small points.
Here's a function I've been running for a while that does something similar.
I didn't look in detail with your code, so I can't offer a feature comparison.
It also offers O(1)-ish lookup time, whereas from a brief read it looks like
your code will be at least O(n).
declare -A JUMP_DIRS
jump() {
(( $# == 1 )) || return 1
dir="${JUMP_DIRS[$1]}"
if ! [[ $dir ]]; then
printf 'No such jump name: %s\n' "$1" >&2
return 1
fi
cd "$dir"
}
You use it by putting `JUMP_DIRS["foo"]=bar' directives in your configuration,
and then jump around using `jump foo'. I cannot think of a time where it was
useful for me to add paths during runtime, so an --add or --remove option seems
pointless.
Best,
Chris
pgpWc0nGsrcdP.pgp
Description: PGP signature
