From: Felipe Contreras Date: Wed, 12 Dec 2012 22:30:40 GMT Subject: Re: [PATCH] completion: add option --recurse-submodules to "git push" Message-ID: In-Reply-To: <7vehj1ixr6.fsf@alter.siamese.dyndns.org> On Fri, Dec 7, 2012 at 11:21 AM, Junio C Hamano wrote: > Steffen Jaeckel writes: > >> Signed-off-by: Steffen Jaeckel >> --- >> contrib/completion/git-completion.bash | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash >> index 0b77eb1..5b4d2e1 100644 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -1434,6 +1434,10 @@ _git_pull () >> __git_complete_remote_or_refspec >> } >> >> +__git_push_recurse_submodules_options=" >> + check on-demand >> +" > > Most of the existing completion functions do not seem to define > separate variables like this; instead, they literally embed their > choices at the point of use. > > Is it expected that the same set of choices will appear in the > completion of many other subcommand options? [jc: Cc'ed Heiko so > that we can sanity check the answer to this question]. If so, the > variable may be justified; otherwise, not. > >> _git_push () >> { >> case "$prev" in >> @@ -1446,10 +1450,15 @@ _git_push () >> __gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}" >> return >> ;; >> + --recurse-submodules=*) >> + __gitcomp "$__git_push_recurse_submodules_options" "" "${cur##--recurse-submodules=}" >> + return >> + ;; > > Owners of the completion script, does this look reasonable? > [jc: Cc'ed Felipe for this] > > This is a tangent, but why is it a double-hash "##" not a > single-hash "#", other than "because all others use ##"? Seems OK by me, but I agree, the options should be inline. -- Felipe Contreras