All of lore.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.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.org \
    /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.