public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Ted Pavlic <ted@tedpavlic•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: "Shawn O. Pearce" <spearce@spearce•org>, git <git@vger•kernel.org>
Subject: Re: [PATCH] Simple update to bash completions to prevent unbound variable errors.
Date: Mon, 12 Jan 2009 23:30:16 -0500	[thread overview]
Message-ID: <496C18D8.9070707@tedpavlic.com> (raw)
In-Reply-To: <7vy6xfew2n.fsf@gitster.siamese.dyndns.org>

>> A vim modeline has also been added for consistency.
>
> Yuck.

Better that than have a mixture of spaces and tabs.

>> +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
>> +# returns text to add to bash PS1 prompt (includes branch name)
>
> Good.  Would be better if you described what $1 and $2 mean.

In that case, there's only $1 (the format string).

Note that in most cases, I didn't know what $1 and $2 were. I was just 
trying to fix it so that it would work on my system.

>> -		if [ -n "$1" ]; then
>> +		if [ $# -gt 0 ]&&  [ -n "$1" ]; then
>
> I found the previous round's [ -n "${1-}" ] much easier to read, if we were to
> do this.  If -n "${1-}", then "$1" is definitely set so nothing need to
> change in the then ... else part.

Hey -- I agree, but no one else liked ${1-}. And hg's bash completion 
seems far superior because they avoid ever having to worry about it. 
They actually thought about the arguments ahead of time.

>> +# __gitcomp_1 requires 2 arguments
>
> ... and $1 and $2 mean?

No clue. Patches are welcome.

> Yuck.  If you are taking advantage of the fact that "local one"
> will bind one to emptiness anyway, can't you do something like:
>
> 	local one=${1-} two=${2-} cur=${3-} four=${4-}

Why even use one, two three, and four then?

I had ${#-} throughout, but I was told that that was ugly. So the best I 
could do was come up with the above mess.

--Ted

-- 
Ted Pavlic <ted@tedpavlic•com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

  parent reply	other threads:[~2009-01-13  4:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-13  2:44 [PATCH] Simple update to bash completions to prevent unbound variable errors Ted Pavlic
2009-01-13  3:14 ` Junio C Hamano
2009-01-13  3:56   ` Boyd Stephen Smith Jr.
2009-01-13  4:34     ` Ted Pavlic
2009-01-13  4:50       ` Ted Pavlic
2009-01-13  6:35         ` Teemu Likonen
2009-01-13  8:01         ` Junio C Hamano
2009-01-13  4:30   ` Ted Pavlic [this message]
2009-01-13  5:37     ` Junio C Hamano

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=496C18D8.9070707@tedpavlic.com \
    --to=ted@tedpavlic$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=spearce@spearce$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox