From: Junio C Hamano <gitster@pobox•com>
To: John Keeping <john@keeping•me.uk>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 1/4] completion: add missing terminator in case statement
Date: Tue, 22 Jul 2014 13:18:17 -0700 [thread overview]
Message-ID: <xmqqvbqpf8qd.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140722182207.GC26927@serenity.lan> (John Keeping's message of "Tue, 22 Jul 2014 19:22:07 +0100")
John Keeping <john@keeping•me.uk> writes:
> On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox•com> writes:
>>
>> > John Keeping <john@keeping•me.uk> writes:
>> >
>> >> Signed-off-by: John Keeping <john@keeping•me.uk>
>> >> ---
>> >
>> > As these ;; are separators not terminators, this is not strictly
>> > necessary. Squashing it into a change that adds more case arms to
>> > this case statement is of course not just good but necessary,
>> > though.
>>
>> s/necessary/may be &/; if you add new arms before this one, you
>> won't need it. But if you add one after this, you would ;-).
>
> Hmm... POSIX describes them as terminators :-)
>
> The compound-list for each list of patterns, with the possible
> exception of the last, shall be terminated with ";;".
A terminator that is optional at the end is a separator ;-).
Having ';;' immediately before 'esac' is not wrong, but omitting it
is exactly equally correct as having one, so it is not something we
would want a patch to churn.
> I'll drop this patch in the re-roll since it isn't necessary.
This round looked good from a cursory read, except that the first
one still makes me wonder why you chose to put it there _before_
where we handle --repo, where the corresponding case on "$cur"
handles --repo= first and then --recurse-submodules= next.
Wouldn't the end result easier to follow if you stuck to the same
order?
next prev parent reply other threads:[~2014-07-22 20:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-19 9:45 [PATCH 1/4] completion: add missing terminator in case statement John Keeping
2014-07-19 9:45 ` [PATCH 2/4] completion: complete "unstuck" `git push --recurse-submodules` John Keeping
2014-07-19 11:37 ` Johannes Sixt
2014-07-19 15:10 ` John Keeping
2014-07-19 9:45 ` [PATCH 3/4] completion: add some missing options to `git push` John Keeping
2014-07-19 9:45 ` [PATCH 4/4] completion: complete `git push --force-with-lease=` John Keeping
2014-07-21 17:26 ` [PATCH 1/4] completion: add missing terminator in case statement Junio C Hamano
2014-07-21 20:09 ` Junio C Hamano
2014-07-22 18:22 ` John Keeping
2014-07-22 20:18 ` Junio C Hamano [this message]
2014-07-22 18:24 ` [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` John Keeping
2014-07-22 18:24 ` [PATCH v2 2/3] completion: add some missing options to `git push` John Keeping
2014-07-22 18:24 ` [PATCH v2 3/3] completion: complete `git push --force-with-lease=` John Keeping
2014-07-22 20:23 ` [PATCH v2 1/3] completion: complete "unstuck" `git push --recurse-submodules` Junio C Hamano
2014-07-22 20:57 ` John Keeping
2014-07-22 21:17 ` Junio C Hamano
2014-07-23 19:35 ` John Keeping
2014-07-23 21:31 ` 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=xmqqvbqpf8qd.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=john@keeping$(echo .)me.uk \
/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