Re: [PATCH v3 5/5] completion: git-bisect view recognized but not completed

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 19, 2024, 07:05 UTC
Message-ID: <ZaofIHEx9frnw5ig@tanuki>
In-Reply-To: <20240118204323.1113859-6-britton.kerin@gmail.com>

On Thu, Jan 18, 2024 at 11:43:23AM -0900, Britton Leo Kerin wrote:

The commit subject is a bit weird in that it does not contain any verb, it only postulates what happens after the change you do here. How about "completion: complete options for git-bisect view"? You can then explain why we only complete options, but not the actual subcommand itself in the commit message a bit more in depth.

Show 21 quoted lines

> This allows the git-log options to be completed but avoids completion
> ambiguity between visualize and view.
> 
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ad80df6630..87cf7b2561 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1591,13 +1591,22 @@ _git_bisect ()
>  		term_good=`__git bisect terms --term-good`
>  	fi
>  
> -	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
> +	# We will complete any custom terms, but still always complete the
> +	# more usual bad/new/good/old because git bisect gives a good error
> +	# message if these are given when not in use and that's better than
> +	# silent refusal to complete if the user is confused.

This part doesn't have a lot to do with the changes you're doing in this commit. So it might've been better to add this explanation in the patch where you introduced completion for "$term_good" and "$term_bad".

Patrick

Show 28 quoted lines

> +	# We want to recognize 'view' but not complete it, because it overlaps
> +	# with 'visualize' too much and is just an alias for it.
> +	#
> +	local completable_subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
> +	local all_subcommands="$completable_subcommands view"
>  
> -	local subcommand="$(__git_find_on_cmdline "$subcommands")"
> +	local subcommand="$(__git_find_on_cmdline "$all_subcommands")"
>  
>  	if [ -z "$subcommand" ]; then
>  		if [ -f "$__git_repo_path"/BISECT_START ]; then
> -			__gitcomp "$subcommands"
> +			__gitcomp "$completable_subcommands"
>  		else
>  			__gitcomp "replay start"
>  		fi
> @@ -1615,7 +1624,7 @@ _git_bisect ()
>  			;;
>  		esac
>  		;;
> -	visualize)
> +	visualize|view)
>  		case "$cur" in
>  		-*)
>  			__git_complete_log_opts
> -- 
> 2.43.0
>