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

From: John Keeping <[email-masked]>
Date: Tue, 22 Jul 2014 19:22:07 +0100

On Mon, Jul 21, 2014 at 01:09:13PM -0700, Junio C Hamano wrote:
> Junio C Hamano <[email-masked]> writes:
> 
> > John Keeping <[email-masked]> writes:
> >
> >> Signed-off-by: John Keeping <[email-masked]>
> >> ---
> >
> > 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 ";;".

Although, bash.info is inaccurate here (clearly Bash does implement the
POSIX behaviour otherwise the existing code wouldn't work):

	Each clause must be terminated with `;;', `;&', or `;;&'.  The
	WORD undergoes tilde expansion, parameter expansion, command
	substitution, arithmetic expansion, and quote removal before
	matching is attempted.  Each PATTERN undergoes tilde expansion,
	parameter expansion, command substitution, and arithmetic
	expansion.

	There may be an arbitrary number of `case' clauses, each terminated
	by a `;;', `;&', or `;;&'.  The first pattern that matches
	determines the command-list that is executed.

I'll drop this patch in the re-roll since it isn't necessary.

> >>  contrib/completion/git-completion.bash | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index 7a6e1d7..d0b2895 100644
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -1627,6 +1627,7 @@ _git_push ()
> >>  	--repo)
> >>  		__gitcomp_nl "$(__git_remotes)"
> >>  		return
> >> +		;;
> >>  	esac
> >>  	case "$cur" in
> >>  	--repo=*)