Re: [PATCH v2 1/4] completion: add 'git history' subcommands

> On 07/08/2026 at 08:30, Patrick Steinhardt wrote:
> > On Thu, Aug 06, 2026 at 10:27:36PM +0200, Vincent Mailhol wrote:
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index e875787710..7372e2919b 100644
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -2137,6 +2137,54 @@ _git_help ()
> >>  	fi
> >>  }
> >>  
> >> +__git_history_has_revision ()
> >> +{
> >> +	local i
> >> +
> >> +	for ((i = __git_cmd_idx + 2; i < cword; i++)); do
> >> +		case "${words[i]}" in
> >> +		--empty|--update-refs)
> >> +			((i++))
> >> +			;;
> > 
> > This will unfortunately be quite a pain to maintain going forward, as we
> > now have to be aware of updating this site every single time we add a
> > new option that accepts a parameter.
> 
> Do you foreseen such new parameters?

> > I don't really have a good idea for how to fix that reliably though, I
> > have to admit. Maybe we should just mostly ignore this edge case and
> > always complete references, unless we have seen a `--`? That can be
> > checked rather easily via `__git_hash_doubledash`.
> 
> My toughs are that if such a special case ever surface, we can just
> dispatch it earlier before we check for the
> __git_history_has_revision, like this:
> 
> ---8<---
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d313780d8b..786fcb5e16 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2193,6 +2193,15 @@ _git_history ()
>  		esac
>  	fi
>  
> +	# Subcommands which takes something else than a revision
> +	case "$subcommand" in
> +	foo)
> +		# 'git history foo' take a file first
> +		__git_complete_index_file "--cached"
> +		return
> +		;;
> +	esac
> +
>  	if ! __git_history_has_revision; then
>  		__git_complete_refs
>  		return
> ---8<---
> 
> This seems reasonable to me. Once we know what this mysterious new
> command would be, maybe we can find a smarter and more tailored
> solution, but at the moment, I would not call this a blocker.