public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce•org>
To: ted@tedpavlic•com
Cc: git@vger•kernel.org, gitster@pobox•com
Subject: Re: [PATCH 1/3] bash-completion: Support running when set -u is enabled
Date: Thu, 15 Jan 2009 07:34:02 -0800	[thread overview]
Message-ID: <20090115153402.GE10179@spearce.org> (raw)
In-Reply-To: <1231963342-24461-1-git-send-email-ted@tedpavlic.com>

ted@tedpavlic•com wrote:
> From: Ted Pavlic <ted@tedpavlic•com>
> 
>   Under "set -u" semantics, it is an error to access undefined
>   variables. Some user environments may enable this setting in the
>   interactive shell.
> 
>   In any context where the completion functions access an undefined
>   variable, accessing a default empty string (aka "${1-}" instead of
>   "$1") is a reasonable way to code the function, as it silences
>   the undefined variable error while still supplying an empty string.
> 
>   In this patch, functions that should always take an argument still use
>   $1. Functions that have optional arguments use ${1-}.
> 
> Signed-off-by: Ted Pavlic <ted@tedpavlic•com>
> Acked-by: Shawn O. Pearce <spearce@spearce•org>

Yup.  This patch isn't as bad as everyone made it out to be.
I think we should go ahead and include it.

But usually we don't indent the commit message like you did
above; instead the message should be aligned on the left margin at
column 0.  git log will indent the message during display, hence
any identation you add in the patch email just makes the message
that much harder to read from git log.

> ---
>  contrib/completion/git-completion.bash |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)

-- 
Shawn.

      parent reply	other threads:[~2009-01-15 15:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-14 20:02 [PATCH 1/3] bash-completion: Support running when set -u is enabled ted
2009-01-14 20:02 ` [PATCH 2/3] bash-completion: Try bash completions before simple filetype ted
2009-01-14 20:02   ` [PATCH 3/3] bash-completion: Added comments to remind about required arguments ted
2009-01-15 15:35     ` Shawn O. Pearce
2009-01-15 15:47       ` Ted Pavlic
2009-01-15 15:34 ` Shawn O. Pearce [this message]

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