On Fri, Dec 7, 2012 at 11:21 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Steffen Jaeckel <steffen.jaeckel@xxxxxxxxx> writes:
>
>> Signed-off-by: Steffen Jaeckel <steffen.jaeckel@xxxxxxxxx>
>> ---
>> 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
--
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