Re: to - Bookmark file system locations in bash on POSIX-like systems

[Chris Down]

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

pgp05Sz1xRvvX.pgp
Description: PGP signature