From: Junio C Hamano <gitster@pobox.com>
To: Ted Pavlic <ted@tedpavlic.com>
Cc: "Boyd Stephen Smith Jr." <bss@iguanasuicide.net>,
"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: Tue, 13 Jan 2009 00:01:08 -0800 [thread overview]
Message-ID: <7vab9veit7.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <496C1D90.1020609@tedpavlic.com> (Ted Pavlic's message of "Mon, 12 Jan 2009 23:50:24 -0500")
Ted Pavlic <ted@tedpavlic.com> writes:
> An alternative (to a Vim modeline) is to put
>
> #!bash
I actually like this much better than "# vim:" thing.
It talks about WHAT the file is ("It is a bash scriptlet"), as opposed to
"# vim:" that tells HOW one user wants to edit the file ("with this
settings I use vim to edit this file"). In case somebody misunderstood
me, my "Yuck" was not about Emacs vs vi holy war. [*1*].
As people noted, it may not have any meaning to the shell, but it serves
as an important documentation to humans. As the maintainer, I try to be
reasonably strict about keeping our shell scripts free of bash-ism to help
people on platforms without bash, but this file is about bash (and bash
only). We are free to use all bash-isms we usually try to stay away from
in our scripts marked with #!/bin/sh (namely, "local", "let", shell
arrays, and substitutions outside POSIX such as ${parameter:offset:length}
substring, ${#parameter} length, and ${parameter/pattern/string} regexp
replacement), as long as they are compatible across recent bash versions.
If tools like vim notice the same hint that says "this is bash" and
adjusts its behaviour accordingly, that is great.
Even though I've grown to know Shawn well enough to be able to tell that
certain kinds of changes would get his Ack and often apply patches myself
without waiting him to give his Ack, the completion script is ultimately
his code and you do not necessarily have to make me like it.
Your updated patch still had [ $# -eq 0 -o -z "$1" ] even though the
commit log message (which is not the place to describe changes between v1
and v2 submissions, by the way) claimed that you replaced them with
"${1-}". I am guessing it would need at least one more iteration to clean
up, but I think this patch is improving _provided if_ supporting "set -u"
in the user's interactive environment is a good idea to begin with.
I just hope that next person who complains about bash completion scripts
would not say that he has "set -e" in his interactive environment ;-) I
personally consider that "set -u" is equally silly, but it probably is
just me.
[Footnote]
*1* For the same reason, Local Variables section to please Emacs is
equally offending; it tends to be much bigger which is even worse.
next prev parent reply other threads:[~2009-01-13 8:02 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 [this message]
2009-01-13 4:30 ` Ted Pavlic
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=7vab9veit7.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=bss@iguanasuicide.net \
--cc=git@vger.kernel.org \
--cc=spearce@spearce.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).