Hi, On Mon, Apr 11, 2011 at 07:47:52PM +0200, Nicolas Morey-Chaisemartin wrote: > Completion for git submodule only handled subcommands. > Add support for options of each subcommand > > Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@xxxxxxxxxxxxxxxxxxxxxx> > --- > Changes: > - Inlined local variables listing options. > > contrib/completion/git-completion.bash | 32 +++++++++++++++++++++++++++++++- > 1 files changed, 31 insertions(+), 1 deletions(-) This change looks good overall, but I have a few nits, though. > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 840ae38..20d0cf0 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2546,7 +2546,8 @@ _git_submodule () > __git_has_doubledash && return > > local subcommands="add status init update summary foreach sync" > - if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then > + local subcommand="$(__git_find_on_cmdline "$subcommands")" > + if [ -z "$subcommand" ]; then > local cur > _get_comp_words_by_ref -n =: cur > case "$cur" in > @@ -2558,6 +2559,35 @@ _git_submodule () > ;; > esac > return > + else > + local cur > + _get_comp_words_by_ref -n =: cur Both branches of the if-else statement start with the same local cur _get_comp_words_by_ref -n =: cur so these could be moved out in front of the if statement. > + case "$subcommand,$cur" in > + add,--*) > + __gitcomp "--branch= --force --reference=" > + ;; > + status,--*) > + __gitcomp "--cached --recursive" > + ;; > + update,--*) > + __gitcomp "--init --no-fetch --rebase --reference= --merge --recursive" > + ;; > + summary,--*) > + __gitcomp "--cached --files --summary-limit=" > + ;; > + summary,*) > + __gitcomp "$(__git_refs)" > + ;; > + foreach,--*) > + __gitcomp "--recursive" > + ;; > + sync,*) > + COMPREPLY=() > + ;; > + *) > + COMPREPLY=() > + ;; The 'sync,*' case is not necessary, because it has the same completion reply as the '*' case, and that case would match it anyway. And finally, please use tabs for indentation. Best, Gábor -- 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