From: "Shawn O. Pearce" <spearce@spearce.org>
To: Ted Pavlic <ted@tedpavlic.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] Simplest update to bash completions to prevent unbounded variable errors
Date: Tue, 13 Jan 2009 07:20:47 -0800 [thread overview]
Message-ID: <20090113152047.GO10179@spearce.org> (raw)
In-Reply-To: <496C1F5F.9020604@tedpavlic.com>
Ted Pavlic <ted@tedpavlic.com> wrote:
> Another try at fixing bash completions in "set -u" environments.
I agree with Junio; setting -u in your interactive shell is as bad
as export CDPATH. Its crazy.
> Additionally added some comments and omitted things like Vim modelines.
These are orthogonal to the -u corrections. They should be in a
different patch. The comments are wecome. The '#!bash' looks like
a good idea. But a vim specific modeline, I don't like, for the
reasons Junio has already stated.
> +# __gitdir accepts 0 or 1 arguments (i.e., location)
> +# returns location of .git repo
> __gitdir ()
> {
> - if [ -z "$1" ]; then
> + if [ $# -eq 0 ] || [ -z "$1" ]; then
This is one of those places where [ -z "${1-}" ] is likely easier
to read then the || usage you have introduced. We don't care if
we got no args, or we got one that is the empty string, either way
the $1 cannot be a gitdir and we need to guess it.
> @@ -111,7 +116,7 @@ __git_ps1 ()
> fi
> fi
>
> - if [ -n "$1" ]; then
> + if [ $# -gt 0 ] && [ -n "$1" ]; then
> printf "$1" "${b##refs/heads/}$r"
Eh, I'd rather see [ -n "${1-}" ] over the && test.
> -complete -o default -o nospace -F _git git
> -complete -o default -o nospace -F _gitk gitk
> +complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
> + || complete -o default -o nospace -F _git git
> +complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
> + || complete -o default -o nospace -F _gitk gitk
Why are we switching to bashdefault? Is this an unrelated change
from the -u stuff and should go into its own commit, with its own
justification?
--
Shawn.
next prev parent reply other threads:[~2009-01-13 15:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-13 4:58 [PATCH] Simplest update to bash completions to prevent unbounded variable errors Ted Pavlic
2009-01-13 15:20 ` Shawn O. Pearce [this message]
2009-01-13 15:30 ` Ted Pavlic
2009-01-13 15:33 ` Shawn O. Pearce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090113152047.GO10179@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ted@tedpavlic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.