Re: [PATCH 1/4] completion: add missing terminator in case statement

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



John Keeping <john@xxxxxxxxxxxxx> writes:

> On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>> 
>> > John Keeping <john@xxxxxxxxxxxxx> writes:
>> >
>> >> Signed-off-by: John Keeping <john@xxxxxxxxxxxxx>
>> >> ---
>> >
>> > 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?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]