On Thu, Jan 18, 2024 at 11:43:20AM -0900, Britton Leo Kerin wrote:
Proposal for the commit subject: "completion: complete log opts for
git-bisect visualize".
> To do this the majority of _git_log has been factored out into the new
> __git_complete_log_opts.
We typically do not continue the commit message as if the commit subject
was the first line of the message. An introduction like the following
would help to set the stage:
Arguments passed to the "visualize" subcommand of git-bisect(1) get
forwarded to git-log(1). It thus supports the same options as
git-log(1) would, but our Bash completion script does not know to
handle this.
> This is needed because the visualize command
> accepts git-log options but not rev arguments (they are fixed to the
> commits under bisection).
>
> __git_complete_log_opts has a precondition that COMPREPLY be empty. In
> a completion context it doesn't seem advisable to implement
> preconditions as noisy or hard failures, so instead it becomes a no-op
> on violation. This should be detectable and quick to debug for devels,
> without ever aggravating a user (besides completion failure).
>
> Signed-off-by: Britton Leo Kerin <britton.kerin@xxxxxxxxx>
> ---
> contrib/completion/git-completion.bash | 30 +++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 15d22ff7d9..c16aded36c 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1472,6 +1472,16 @@ _git_bisect ()
> ;;
> esac
> ;;
> + visualize)
> + case "$cur" in
> + -*)
> + __git_complete_log_opts
> + return
> + ;;
> + *)
> + ;;
> + esac
> + ;;
Is this switch even needed? Can't we call `__git_complete_log_opts`
directly?
> esac
>
> case "$subcommand" in
> @@ -2074,10 +2084,14 @@ __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-c
> __git_log_pretty_formats="oneline short medium full fuller reference email raw format: tformat: mboxrd"
> __git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
>
> -_git_log ()
> +
> +# Check for only porcelain (i.e. not git-rev-list) option (not argument)
> +# and selected option argument completions for git-log options and if any
> +# are found put them in COMPREPLY. COMPREPLY must be empty at the start,
> +# and will be empty on return if no candidates are found.
Why do we need to enforce that COMPREPLY is empty? None of the other
`__git_complete_*` helpers do this, so I think it's fair to expect that
the variable woulld get clobbered when calling the new function. Thus, I
don't think there's a need for this precondition.
> +__git_complete_log_opts ()
> {
> - __git_has_doubledash && return
> - __git_find_repo_path
> + [ -z "$COMPREPLY" ] || return 1 # Precondition
>
> local merge=""
> if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> @@ -2171,6 +2185,16 @@ _git_log ()
> return
> ;;
> esac
> +}
> +
> +_git_log ()
> +{
> + __git_has_doubledash && return
> + __git_find_repo_path
I was about to say that it would make more sense to call
`__git_find_repo_path` in `__git_complete_log_opts` so that all prereqs
are fulfilled whenever the latter is called. But `__git_complete_relist`
doesn't know to find the repo path in all cases, so that wouldn't quite
work alright.
Patrick
> + __git_complete_log_opts
> + [ -z "$COMPREPLY" ] || return
> +
> __git_complete_revlist
> }
>
> --
> 2.43.0
>
Attachment:
signature.asc
Description: PGP signature
