[RFC PATCH 0/6] completion: improvements for git-bisect

[RFC PATCH 0/6] completion: improvements for git-bisect

[Britton Leo Kerin]

Bring completion for bisect up to date with current features.

This is RFC mainly because of an implementation issue in patch 2: I think
the mechanism of requiring COMPREPLY to be empty, possibly writing into it,
then checking it post-call is too fragile, but I'm not sure how people would
like to fix this.  I'd prefer to just add an assert() of some sort to enforce
the empty-COMPREPLY precondition, rather than rewrite the guts of the former
_git_log() function to use some other intermediate return mechanism (this
would be overkill in this simple context IMO).  But I'm not sure how to
do that in a completion context: for sure nothing that is done here should
ever have a chance of showing garbage on a user's screen, but of course for
assert() to be useful a dev would need to see it somehow.  Suggestions welcome.

Other than that issue I think it's all reasonable and ready, though there
are a couple other decisions that I guess people might disagree with:

  * good/old/new/bad terms are always completed (even if they're
    invalild because --term-(good|bad) have been used).  The idea here
    is that if the user has become confused about their own terms it's
    better to let git give them an error message than to have
    normally-working completion not happen.

  * 'view' is recognized as a subcommand, but not as a completion
    candidate.  This lets complete of git bisect v<TAB> keep working which
    seems convenient and some poor person somewhere probably really wants :)
    view is just an alias so loss of interface recall is minimal.

Britton Leo Kerin (6):
  completion: complete new old actions, start opts
  completion: git-log opts to bisect visualize
  completion: move to maintain define-before-use
  completion: custom git-bisect terms
  completion: recognize but do not complete 'view'
  completion: add comment

 contrib/completion/git-completion.bash | 310 +++++++++++++++----------
 1 file changed, 181 insertions(+), 129 deletions(-)


base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.43.0

[PATCH v2 0/5] completion: improvements for git-bisect

[Britton Leo Kerin]

Bring completion for bisect up to date with current features.

I didn't hear back on my previous RFC Patch for these improvements, and
ultimately decided that simply aborting special completion efforts and
doing nothing on precondition violation is the best policy, because devs
will likely notice and users won't be irritated more than necessary.

Besides that the series has just been cleaned up a tiny bit.

Britton Leo Kerin (5):
  completion: complete new old actions, start opts
  completion: git-log opts to bisect visualize
  completion: move to maintain define-before-use
  completion: custom git-bisect terms
  completion: custom git-bisect terms

 contrib/completion/git-completion.bash | 312 +++++++++++++++----------
 1 file changed, 183 insertions(+), 129 deletions(-)

Range-diff against v1:
1:  e16264bfb9 = 1:  e16264bfb9 completion: complete new old actions, start opts
2:  90466cdfa5 ! 2:  130abe3460 completion: git-log opts to bisect visualize
    @@ Commit message
         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@gmail.com>

      ## contrib/completion/git-completion.bash ##
    @@ contrib/completion/git-completion.bash: __git_diff_merges_opts="off none on firs
      __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.
    ++__git_complete_log_opts ()
    + {
     -	__git_has_doubledash && return
     -	__git_find_repo_path
    ++	[ -z "$COMPREPLY" ] || return 1   # Precondition

    -+# Find only porcelain (i.e. not git-rev-list) option (not argument) and
    -+# selected option argument completions for git-log options and put them in
    -+# COMPREPLY.
    -+__git_complete_log_opts ()
    -+{
      	local merge=""
      	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
    - 		merge="--merge"
     @@ contrib/completion/git-completion.bash: _git_log ()
      		return
      		;;
      	esac
    -+
     +}
     +
     +_git_log ()
3:  0edfb54dd5 ! 3:  d659ace9c2 completion: move to maintain define-before-use
    @@ contrib/completion/git-completion.bash: _git_archive ()
     +__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:"
     +
    -+# Find only porcelain (i.e. not git-rev-list) option (not argument) and
    -+# selected option argument completions for git-log options and put them in
    -+# COMPREPLY.
    ++# 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.
     +__git_complete_log_opts ()
     +{
    ++	[ -z "$COMPREPLY" ] || return 1   # Precondition
    ++
     +	local merge=""
     +	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
     +		merge="--merge"
    @@ contrib/completion/git-completion.bash: _git_archive ()
     +		return
     +		;;
     +	esac
    -+
     +}
     +
      _git_bisect ()
    @@ contrib/completion/git-completion.bash: _git_ls_tree ()
     -__git_log_date_formats="relative iso8601 iso8601-strict rfc2822 short local default human raw unix auto: format:"
     -
     -
    --# Find only porcelain (i.e. not git-rev-list) option (not argument) and
    --# selected option argument completions for git-log options and put them in
    --# COMPREPLY.
    +-# 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.
     -__git_complete_log_opts ()
     -{
    +-	[ -z "$COMPREPLY" ] || return 1   # Precondition
    +-
     -	local merge=""
     -	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
     -		merge="--merge"
    @@ contrib/completion/git-completion.bash: _git_ls_tree ()
     -		return
     -		;;
     -	esac
    --
     -}
     -
      _git_log ()
4:  a8a730ffbe = 4:  c5bee633b2 completion: custom git-bisect terms
5:  44d3a36e20 ! 5:  220650f0ba completion: recognize but do not complete 'view'
    @@ Metadata
     Author: Britton Leo Kerin <britton.kerin@gmail.com>

      ## Commit message ##
    -    completion: recognize but do not complete 'view'
    -
    -    Completing it might annoy some existing users by creating completion
    -    ambiguity on 'v' and 'vi' without adding anything useful in terms of
    -    interface discovery/recall (because 'view' is just an alias anyway).
    +    completion: custom git-bisect terms

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

    @@ contrib/completion/git-completion.bash: _git_bisect ()
      	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.
    ++	#
     +	# 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"

6:  7b88549fbe < -:  ---------- completion: add comment
--
2.43.0

[PATCH v3 0/5] completion: improvements for git-bisect

[Britton Leo Kerin]

CC to add: CC: Junio C Hamano <gitster@pobox.com>, Patrick Steinhardt <ps@pks.im>

Relative to v2 this only changes a wrong misleading commit message.

Bring completion for bisect up to date with current features.

Britton Leo Kerin (5):
  completion: complete new old actions, start opts
  completion: git-log opts to bisect visualize
  completion: move to maintain define-before-use
  completion: custom git-bisect terms
  completion: git-bisect view recognized but not completed

 contrib/completion/git-completion.bash | 312 +++++++++++++++----------
 1 file changed, 183 insertions(+), 129 deletions(-)

Range-diff against v2:
1:  e16264bfb9 = 1:  e16264bfb9 completion: complete new old actions, start opts
2:  130abe3460 = 2:  130abe3460 completion: git-log opts to bisect visualize
3:  d659ace9c2 = 3:  d659ace9c2 completion: move to maintain define-before-use
4:  c5bee633b2 = 4:  c5bee633b2 completion: custom git-bisect terms
5:  220650f0ba ! 5:  2bd0cb26f1 completion: custom git-bisect terms
    @@ Metadata
     Author: Britton Leo Kerin <britton.kerin@gmail.com>

      ## Commit message ##
    -    completion: custom git-bisect terms
    +    completion: git-bisect view recognized but not completed
    +
    +    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>

--
2.43.0

[PATCH v3 1/5] completion: complete new old actions, start opts

[Britton Leo Kerin]

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..15d22ff7d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad good skip reset visualize replay log run"
+	local subcommands="start bad new good old terms skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1462,20 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|good|reset|skip|start)
+	start)
+		case "$cur" in
+		--*)
+			__gitcomp "--term-new --term-bad --term-old --term-good --first-parent --no-checkout"
+			return
+			;;
+		*)
+			;;
+		esac
+		;;
+	esac
+
+	case "$subcommand" in
+	bad|new|good|old|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
-- 
2.43.0

Re: [PATCH v3 1/5] completion: complete new old actions, start opts

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2198 bytes --]

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

I feel like the commit message can be improved. First, you don't mention
that you're adding completion for git-bisect(1), which one needs to
infer from the patch. Second, the message is missing an explanation what
is wrong about the current code and how you're fixing it.

> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 185b47d802..15d22ff7d9 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1449,7 +1449,7 @@ _git_bisect ()
>  {
>  	__git_has_doubledash && return
>  
> -	local subcommands="start bad good skip reset visualize replay log run"
> +	local subcommands="start bad new good old terms skip reset visualize replay log run help"

I'd propose to move the addition of subcommands into one or more
additional commits. "bad", "old" and "help" all already work perfectly
fine, so these can easily go into a single commit. But I think that it
would make sense to introduce "terms" in a standalone commit and then
extend the below case statement to handle its argumens "--term-good" and
"--term-bad".

>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
>  	if [ -z "$subcommand" ]; then
>  		__git_find_repo_path
> @@ -1462,7 +1462,20 @@ _git_bisect ()
>  	fi
>  
>  	case "$subcommand" in
> -	bad|good|reset|skip|start)
> +	start)
> +		case "$cur" in
> +		--*)
> +			__gitcomp "--term-new --term-bad --term-old --term-good --first-parent --no-checkout"
> +			return
> +			;;
> +		*)
> +			;;

If you also called `__git_complete_refs here` then you could merge the
two case statements for "$subcommand". It's a bit of duplicate code, but
I think the end result would be easier to read.

Patrick

> +		esac
> +		;;
> +	esac
> +
> +	case "$subcommand" in
> +	bad|new|good|old|reset|skip|start)
>  		__git_complete_refs
>  		;;
>  	*)
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 2/5] completion: git-log opts to bisect visualize

[Britton Leo Kerin]

To do this the majority of _git_log has been factored out into the new
__git_complete_log_opts.  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@gmail.com>
---
 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
+		;;
 	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.
+__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
+
+        __git_complete_log_opts
+        [ -z "$COMPREPLY" ] || return
+
 	__git_complete_revlist
 }
 
-- 
2.43.0

Re: [PATCH v3 2/5] completion: git-log opts to bisect visualize

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3728 bytes --]

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@gmail.com>
> ---
>  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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 3/5] completion: move to maintain define-before-use

[Britton Leo Kerin]

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 269 ++++++++++++-------------
 1 file changed, 134 insertions(+), 135 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c16aded36c..63ca8082a4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1445,6 +1445,140 @@ _git_archive ()
 	__git_complete_file
 }
 
+# Options that go well for log, shortlog and gitk
+__git_log_common_options="
+	--not --all
+	--branches --tags --remotes
+	--first-parent --merges --no-merges
+	--max-count=
+	--max-age= --since= --after=
+	--min-age= --until= --before=
+	--min-parents= --max-parents=
+	--no-min-parents --no-max-parents
+"
+# Options that go well for log and gitk (not shortlog)
+__git_log_gitk_options="
+	--dense --sparse --full-history
+	--simplify-merges --simplify-by-decoration
+	--left-right --notes --no-notes
+"
+# Options that go well for log and shortlog (not gitk)
+__git_log_shortlog_options="
+	--author= --committer= --grep=
+	--all-match --invert-grep
+"
+# Options accepted by log and show
+__git_log_show_options="
+	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
+"
+
+__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
+
+__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:"
+
+# 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.
+__git_complete_log_opts ()
+{
+	[ -z "$COMPREPLY" ] || return 1   # Precondition
+
+	local merge=""
+	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
+		merge="--merge"
+	fi
+	case "$prev,$cur" in
+	-L,:*:*)
+		return	# fall back to Bash filename completion
+		;;
+	-L,:*)
+		__git_complete_symbol --cur="${cur#:}" --sfx=":"
+		return
+		;;
+	-G,*|-S,*)
+		__git_complete_symbol
+		return
+		;;
+	esac
+	case "$cur" in
+	--pretty=*|--format=*)
+		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
+			" "" "${cur#*=}"
+		return
+		;;
+	--date=*)
+		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
+		return
+		;;
+	--decorate=*)
+		__gitcomp "full short no" "" "${cur##--decorate=}"
+		return
+		;;
+	--diff-algorithm=*)
+		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+		return
+		;;
+	--submodule=*)
+		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
+		return
+		;;
+	--ws-error-highlight=*)
+		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
+		return
+		;;
+	--no-walk=*)
+		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
+		return
+		;;
+	--diff-merges=*)
+                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
+                return
+                ;;
+	--*)
+		__gitcomp "
+			$__git_log_common_options
+			$__git_log_shortlog_options
+			$__git_log_gitk_options
+			$__git_log_show_options
+			--root --topo-order --date-order --reverse
+			--follow --full-diff
+			--abbrev-commit --no-abbrev-commit --abbrev=
+			--relative-date --date=
+			--pretty= --format= --oneline
+			--show-signature
+			--cherry-mark
+			--cherry-pick
+			--graph
+			--decorate --decorate= --no-decorate
+			--walk-reflogs
+			--no-walk --no-walk= --do-walk
+			--parents --children
+			--expand-tabs --expand-tabs= --no-expand-tabs
+			$merge
+			$__git_diff_common_options
+			"
+		return
+		;;
+	-L:*:*)
+		return	# fall back to Bash filename completion
+		;;
+	-L:*)
+		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
+		return
+		;;
+	-G*)
+		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
+		return
+		;;
+	-S*)
+		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
+		return
+		;;
+	esac
+}
+
 _git_bisect ()
 {
 	__git_has_doubledash && return
@@ -2052,141 +2186,6 @@ _git_ls_tree ()
 	__git_complete_file
 }
 
-# Options that go well for log, shortlog and gitk
-__git_log_common_options="
-	--not --all
-	--branches --tags --remotes
-	--first-parent --merges --no-merges
-	--max-count=
-	--max-age= --since= --after=
-	--min-age= --until= --before=
-	--min-parents= --max-parents=
-	--no-min-parents --no-max-parents
-"
-# Options that go well for log and gitk (not shortlog)
-__git_log_gitk_options="
-	--dense --sparse --full-history
-	--simplify-merges --simplify-by-decoration
-	--left-right --notes --no-notes
-"
-# Options that go well for log and shortlog (not gitk)
-__git_log_shortlog_options="
-	--author= --committer= --grep=
-	--all-match --invert-grep
-"
-# Options accepted by log and show
-__git_log_show_options="
-	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
-"
-
-__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
-
-__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:"
-
-
-# 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.
-__git_complete_log_opts ()
-{
-	[ -z "$COMPREPLY" ] || return 1   # Precondition
-
-	local merge=""
-	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
-		merge="--merge"
-	fi
-	case "$prev,$cur" in
-	-L,:*:*)
-		return	# fall back to Bash filename completion
-		;;
-	-L,:*)
-		__git_complete_symbol --cur="${cur#:}" --sfx=":"
-		return
-		;;
-	-G,*|-S,*)
-		__git_complete_symbol
-		return
-		;;
-	esac
-	case "$cur" in
-	--pretty=*|--format=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur#*=}"
-		return
-		;;
-	--date=*)
-		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
-		return
-		;;
-	--decorate=*)
-		__gitcomp "full short no" "" "${cur##--decorate=}"
-		return
-		;;
-	--diff-algorithm=*)
-		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
-		return
-		;;
-	--submodule=*)
-		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
-		return
-		;;
-	--ws-error-highlight=*)
-		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
-		return
-		;;
-	--no-walk=*)
-		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
-		return
-		;;
-	--diff-merges=*)
-                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
-                return
-                ;;
-	--*)
-		__gitcomp "
-			$__git_log_common_options
-			$__git_log_shortlog_options
-			$__git_log_gitk_options
-			$__git_log_show_options
-			--root --topo-order --date-order --reverse
-			--follow --full-diff
-			--abbrev-commit --no-abbrev-commit --abbrev=
-			--relative-date --date=
-			--pretty= --format= --oneline
-			--show-signature
-			--cherry-mark
-			--cherry-pick
-			--graph
-			--decorate --decorate= --no-decorate
-			--walk-reflogs
-			--no-walk --no-walk= --do-walk
-			--parents --children
-			--expand-tabs --expand-tabs= --no-expand-tabs
-			$merge
-			$__git_diff_common_options
-			"
-		return
-		;;
-	-L:*:*)
-		return	# fall back to Bash filename completion
-		;;
-	-L:*)
-		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
-		return
-		;;
-	-G*)
-		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
-		return
-		;;
-	-S*)
-		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
-		return
-		;;
-	esac
-}
-
 _git_log ()
 {
 	__git_has_doubledash && return
-- 
2.43.0

Re: [PATCH v3 3/5] completion: move to maintain define-before-use

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 9319 bytes --]

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

Same here: please explain what the problem is that this patch is trying
to solve in the commit message.

Also, as far as I can see, this patch is actually a prerequisite for the
preceding patch where we already call `__git_complete_log_opts ()`. So a
better way to structure this would be to introduce and move
`__git_complete_log_opts ()` in a preparatory patch, and only then start
calling the function for "visualize" in a subsequent patch.

Patrick

> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 269 ++++++++++++-------------
>  1 file changed, 134 insertions(+), 135 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c16aded36c..63ca8082a4 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1445,6 +1445,140 @@ _git_archive ()
>  	__git_complete_file
>  }
>  
> +# Options that go well for log, shortlog and gitk
> +__git_log_common_options="
> +	--not --all
> +	--branches --tags --remotes
> +	--first-parent --merges --no-merges
> +	--max-count=
> +	--max-age= --since= --after=
> +	--min-age= --until= --before=
> +	--min-parents= --max-parents=
> +	--no-min-parents --no-max-parents
> +"
> +# Options that go well for log and gitk (not shortlog)
> +__git_log_gitk_options="
> +	--dense --sparse --full-history
> +	--simplify-merges --simplify-by-decoration
> +	--left-right --notes --no-notes
> +"
> +# Options that go well for log and shortlog (not gitk)
> +__git_log_shortlog_options="
> +	--author= --committer= --grep=
> +	--all-match --invert-grep
> +"
> +# Options accepted by log and show
> +__git_log_show_options="
> +	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
> +"
> +
> +__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
> +
> +__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:"
> +
> +# 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.
> +__git_complete_log_opts ()
> +{
> +	[ -z "$COMPREPLY" ] || return 1   # Precondition
> +
> +	local merge=""
> +	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> +		merge="--merge"
> +	fi
> +	case "$prev,$cur" in
> +	-L,:*:*)
> +		return	# fall back to Bash filename completion
> +		;;
> +	-L,:*)
> +		__git_complete_symbol --cur="${cur#:}" --sfx=":"
> +		return
> +		;;
> +	-G,*|-S,*)
> +		__git_complete_symbol
> +		return
> +		;;
> +	esac
> +	case "$cur" in
> +	--pretty=*|--format=*)
> +		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
> +			" "" "${cur#*=}"
> +		return
> +		;;
> +	--date=*)
> +		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
> +		return
> +		;;
> +	--decorate=*)
> +		__gitcomp "full short no" "" "${cur##--decorate=}"
> +		return
> +		;;
> +	--diff-algorithm=*)
> +		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
> +		return
> +		;;
> +	--submodule=*)
> +		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
> +		return
> +		;;
> +	--ws-error-highlight=*)
> +		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
> +		return
> +		;;
> +	--no-walk=*)
> +		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
> +		return
> +		;;
> +	--diff-merges=*)
> +                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
> +                return
> +                ;;
> +	--*)
> +		__gitcomp "
> +			$__git_log_common_options
> +			$__git_log_shortlog_options
> +			$__git_log_gitk_options
> +			$__git_log_show_options
> +			--root --topo-order --date-order --reverse
> +			--follow --full-diff
> +			--abbrev-commit --no-abbrev-commit --abbrev=
> +			--relative-date --date=
> +			--pretty= --format= --oneline
> +			--show-signature
> +			--cherry-mark
> +			--cherry-pick
> +			--graph
> +			--decorate --decorate= --no-decorate
> +			--walk-reflogs
> +			--no-walk --no-walk= --do-walk
> +			--parents --children
> +			--expand-tabs --expand-tabs= --no-expand-tabs
> +			$merge
> +			$__git_diff_common_options
> +			"
> +		return
> +		;;
> +	-L:*:*)
> +		return	# fall back to Bash filename completion
> +		;;
> +	-L:*)
> +		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
> +		return
> +		;;
> +	-G*)
> +		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
> +		return
> +		;;
> +	-S*)
> +		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
> +		return
> +		;;
> +	esac
> +}
> +
>  _git_bisect ()
>  {
>  	__git_has_doubledash && return
> @@ -2052,141 +2186,6 @@ _git_ls_tree ()
>  	__git_complete_file
>  }
>  
> -# Options that go well for log, shortlog and gitk
> -__git_log_common_options="
> -	--not --all
> -	--branches --tags --remotes
> -	--first-parent --merges --no-merges
> -	--max-count=
> -	--max-age= --since= --after=
> -	--min-age= --until= --before=
> -	--min-parents= --max-parents=
> -	--no-min-parents --no-max-parents
> -"
> -# Options that go well for log and gitk (not shortlog)
> -__git_log_gitk_options="
> -	--dense --sparse --full-history
> -	--simplify-merges --simplify-by-decoration
> -	--left-right --notes --no-notes
> -"
> -# Options that go well for log and shortlog (not gitk)
> -__git_log_shortlog_options="
> -	--author= --committer= --grep=
> -	--all-match --invert-grep
> -"
> -# Options accepted by log and show
> -__git_log_show_options="
> -	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
> -"
> -
> -__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
> -
> -__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:"
> -
> -
> -# 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.
> -__git_complete_log_opts ()
> -{
> -	[ -z "$COMPREPLY" ] || return 1   # Precondition
> -
> -	local merge=""
> -	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> -		merge="--merge"
> -	fi
> -	case "$prev,$cur" in
> -	-L,:*:*)
> -		return	# fall back to Bash filename completion
> -		;;
> -	-L,:*)
> -		__git_complete_symbol --cur="${cur#:}" --sfx=":"
> -		return
> -		;;
> -	-G,*|-S,*)
> -		__git_complete_symbol
> -		return
> -		;;
> -	esac
> -	case "$cur" in
> -	--pretty=*|--format=*)
> -		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
> -			" "" "${cur#*=}"
> -		return
> -		;;
> -	--date=*)
> -		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
> -		return
> -		;;
> -	--decorate=*)
> -		__gitcomp "full short no" "" "${cur##--decorate=}"
> -		return
> -		;;
> -	--diff-algorithm=*)
> -		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
> -		return
> -		;;
> -	--submodule=*)
> -		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
> -		return
> -		;;
> -	--ws-error-highlight=*)
> -		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
> -		return
> -		;;
> -	--no-walk=*)
> -		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
> -		return
> -		;;
> -	--diff-merges=*)
> -                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
> -                return
> -                ;;
> -	--*)
> -		__gitcomp "
> -			$__git_log_common_options
> -			$__git_log_shortlog_options
> -			$__git_log_gitk_options
> -			$__git_log_show_options
> -			--root --topo-order --date-order --reverse
> -			--follow --full-diff
> -			--abbrev-commit --no-abbrev-commit --abbrev=
> -			--relative-date --date=
> -			--pretty= --format= --oneline
> -			--show-signature
> -			--cherry-mark
> -			--cherry-pick
> -			--graph
> -			--decorate --decorate= --no-decorate
> -			--walk-reflogs
> -			--no-walk --no-walk= --do-walk
> -			--parents --children
> -			--expand-tabs --expand-tabs= --no-expand-tabs
> -			$merge
> -			$__git_diff_common_options
> -			"
> -		return
> -		;;
> -	-L:*:*)
> -		return	# fall back to Bash filename completion
> -		;;
> -	-L:*)
> -		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
> -		return
> -		;;
> -	-G*)
> -		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
> -		return
> -		;;
> -	-S*)
> -		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
> -		return
> -		;;
> -	esac
> -}
> -
>  _git_log ()
>  {
>  	__git_has_doubledash && return
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 4/5] completion: custom git-bisect terms

[Britton Leo Kerin]

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 63ca8082a4..ad80df6630 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1583,10 +1583,19 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad new good old terms skip reset visualize replay log run help"
+	__git_find_repo_path
+
+	local term_bad term_good
+	if [ -f "$__git_repo_path"/BISECT_START ]; then
+		term_bad=`__git bisect terms --term-bad`
+		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"
+
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
 	if [ -z "$subcommand" ]; then
-		__git_find_repo_path
 		if [ -f "$__git_repo_path"/BISECT_START ]; then
 			__gitcomp "$subcommands"
 		else
@@ -1619,7 +1628,7 @@ _git_bisect ()
 	esac
 
 	case "$subcommand" in
-	bad|new|good|old|reset|skip|start)
+	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
-- 
2.43.0

Re: [PATCH v3 4/5] completion: custom git-bisect terms

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

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

Missing an explanation.

> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 63ca8082a4..ad80df6630 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1583,10 +1583,19 @@ _git_bisect ()
>  {
>  	__git_has_doubledash && return
>  
> -	local subcommands="start bad new good old terms skip reset visualize replay log run help"
> +	__git_find_repo_path
> +
> +	local term_bad term_good
> +	if [ -f "$__git_repo_path"/BISECT_START ]; then
> +		term_bad=`__git bisect terms --term-bad`
> +		term_good=`__git bisect terms --term-good`
> +	fi

We do not use backticks in our codebase. Please use `$(cmd ...)`
instead.

Patrick

> +	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
> +
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
> +
>  	if [ -z "$subcommand" ]; then
> -		__git_find_repo_path
>  		if [ -f "$__git_repo_path"/BISECT_START ]; then
>  			__gitcomp "$subcommands"
>  		else
> @@ -1619,7 +1628,7 @@ _git_bisect ()
>  	esac
>  
>  	case "$subcommand" in
> -	bad|new|good|old|reset|skip|start)
> +	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip|start)
>  		__git_complete_refs
>  		;;
>  	*)
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Britton Leo Kerin]

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.
+	#
+	# 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

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

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]

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.

> 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

> +	# 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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 0/5] completion: improvements for git-bisect

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On Thu, Jan 18, 2024 at 11:43:18AM -0900, Britton Leo Kerin wrote:
> CC to add: CC: Junio C Hamano <gitster@pobox.com>, Patrick Steinhardt <ps@pks.im>
> 
> Relative to v2 this only changes a wrong misleading commit message.
> 
> Bring completion for bisect up to date with current features.

Thanks for your patches! I've got a few comments to bring them more in
line with how we're doing things in the Git project, but I agree with
the overall direction that they are going into.

It might be a good idea to also add a few tests in t9902-completion.sh
to ensure that the newly introduced completions work as expected.

Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 0/5] completion: improvements for git-bisect

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Jan 18, 2024 at 11:43:18AM -0900, Britton Leo Kerin wrote:
>> CC to add: CC: Junio C Hamano <gitster@pobox.com>, Patrick Steinhardt <ps@pks.im>
>> 
>> Relative to v2 this only changes a wrong misleading commit message.
>> 
>> Bring completion for bisect up to date with current features.
>
> Thanks for your patches! I've got a few comments to bring them more in
> line with how we're doing things in the Git project, but I agree with
> the overall direction that they are going into.
>
> It might be a good idea to also add a few tests in t9902-completion.sh
> to ensure that the newly introduced completions work as expected.
>
> Thanks!
>
> Patrick

And thanks for your review.  Very much appreciated.

Re: [PATCH v3 0/5] completion: improvements for git-bisect

[Britton Kerin]

On Thu, Jan 18, 2024 at 10:05 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Jan 18, 2024 at 11:43:18AM -0900, Britton Leo Kerin wrote:
> > CC to add: CC: Junio C Hamano <gitster@pobox.com>, Patrick Steinhardt <ps@pks.im>
> >
> > Relative to v2 this only changes a wrong misleading commit message.
> >
> > Bring completion for bisect up to date with current features.
>
> Thanks for your patches! I've got a few comments to bring them more in
> line with how we're doing things in the Git project, but I agree with
> the overall direction that they are going into.
>
> It might be a good idea to also add a few tests in t9902-completion.sh
> to ensure that the newly introduced completions work as expected.

Thanks for the review, other than the one philosophical issue about how
functions should be introduced/moved I agree with everything you said and
will rework the series accordingly.

Britton

[PATCH v4 0/8] completion: improvements for git-bisect

[Britton Leo Kerin]

Relative to v3 this reworks the commit contents and descriptions
according to review suggestions, removes unnecessary case statements and
precondition, adds option completion for the terms subcommand, and adds
tests.

Britton Leo Kerin (8):
  completion: bisect: complete bad, new, old, and help subcommands
  completion: bisect: complete custom terms and related options
  completion: bisect: complete missing --first-parent and --no-checkout
    options
  completion: new function __git_complete_log_opts
  completion: log: use __git_complete_log_opts
  completion: bisect: complete log opts for visualize subcommand
  completion: bisect: recognize but do not complete view subcommand
  completion: add tests for git-bisect

 contrib/completion/git-completion.bash |  65 ++++++++++--
 t/t9902-completion.sh                  | 135 +++++++++++++++++++++++++
 2 files changed, 193 insertions(+), 7 deletions(-)

Range-diff against v3:
1:  e16264bfb9 ! 1:  66153024c3 completion: complete new old actions, start opts
    @@ Metadata
     Author: Britton Leo Kerin <britton.kerin@gmail.com>

      ## Commit message ##
    -    completion: complete new old actions, start opts
    +    completion: bisect: complete bad, new, old, and help subcommands
    +
    +    The bad, new, old and help subcommands to git-bisect(1) are not
    +    completed.
    +
    +    Add the bad, new, old, and help subcommands to the appropriate lists
    +    such that the commands and their possible ref arguments are completed.

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

    @@ contrib/completion/git-completion.bash: _git_bisect ()
      	__git_has_doubledash && return

     -	local subcommands="start bad good skip reset visualize replay log run"
    -+	local subcommands="start bad new good old terms skip reset visualize replay log run help"
    ++	local subcommands="start bad new good old skip reset visualize replay log run help"
      	local subcommand="$(__git_find_on_cmdline "$subcommands")"
      	if [ -z "$subcommand" ]; then
      		__git_find_repo_path
    @@ contrib/completion/git-completion.bash: _git_bisect ()

      	case "$subcommand" in
     -	bad|good|reset|skip|start)
    -+	start)
    -+		case "$cur" in
    -+		--*)
    -+			__gitcomp "--term-new --term-bad --term-old --term-good --first-parent --no-checkout"
    -+			return
    -+			;;
    -+		*)
    -+			;;
    -+		esac
    -+		;;
    -+	esac
    -+
    -+	case "$subcommand" in
     +	bad|new|good|old|reset|skip|start)
      		__git_complete_refs
      		;;
2:  130abe3460 < -:  ---------- completion: git-log opts to bisect visualize
-:  ---------- > 2:  7eb8c842a3 completion: bisect: complete custom terms and related options
-:  ---------- > 3:  5f5076bb93 completion: bisect: complete missing --first-parent and --no-checkout options
3:  d659ace9c2 ! 4:  c8ffa0e915 completion: move to maintain define-before-use
    @@ Metadata
     Author: Britton Leo Kerin <britton.kerin@gmail.com>

      ## Commit message ##
    -    completion: move to maintain define-before-use
    +    completion: new function __git_complete_log_opts
    +
    +    The options accepted by git-log are also accepted by at least one other
    +    command (git-bisect).  Prepare to factor out the common option and
    +    option argument completion code by defining a new function.

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

      ## contrib/completion/git-completion.bash ##
    -@@ contrib/completion/git-completion.bash: _git_archive ()
    - 	__git_complete_file
    - }
    +@@ contrib/completion/git-completion.bash: __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:"

    -+# Options that go well for log, shortlog and gitk
    -+__git_log_common_options="
    -+	--not --all
    -+	--branches --tags --remotes
    -+	--first-parent --merges --no-merges
    -+	--max-count=
    -+	--max-age= --since= --after=
    -+	--min-age= --until= --before=
    -+	--min-parents= --max-parents=
    -+	--no-min-parents --no-max-parents
    -+"
    -+# Options that go well for log and gitk (not shortlog)
    -+__git_log_gitk_options="
    -+	--dense --sparse --full-history
    -+	--simplify-merges --simplify-by-decoration
    -+	--left-right --notes --no-notes
    -+"
    -+# Options that go well for log and shortlog (not gitk)
    -+__git_log_shortlog_options="
    -+	--author= --committer= --grep=
    -+	--all-match --invert-grep
    -+"
    -+# Options accepted by log and show
    -+__git_log_show_options="
    -+	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
    -+"
    -+
    -+__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
    -+
    -+__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:"
    -+
    -+# 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.
    ++# Complete porcelain (i.e. not git-rev-list) options and at least some
    ++# option arguments accepted by git-log.  Note that this same set of options
    ++# are also accepted by some other git commands besides git-log.
     +__git_complete_log_opts ()
     +{
    -+	[ -z "$COMPREPLY" ] || return 1   # Precondition
    ++        COMPREPLY=""
     +
     +	local merge=""
     +	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
    @@ contrib/completion/git-completion.bash: _git_archive ()
     +	esac
     +}
     +
    - _git_bisect ()
    - {
    - 	__git_has_doubledash && return
    -@@ contrib/completion/git-completion.bash: _git_ls_tree ()
    - 	__git_complete_file
    - }
    -
    --# Options that go well for log, shortlog and gitk
    --__git_log_common_options="
    --	--not --all
    --	--branches --tags --remotes
    --	--first-parent --merges --no-merges
    --	--max-count=
    --	--max-age= --since= --after=
    --	--min-age= --until= --before=
    --	--min-parents= --max-parents=
    --	--no-min-parents --no-max-parents
    --"
    --# Options that go well for log and gitk (not shortlog)
    --__git_log_gitk_options="
    --	--dense --sparse --full-history
    --	--simplify-merges --simplify-by-decoration
    --	--left-right --notes --no-notes
    --"
    --# Options that go well for log and shortlog (not gitk)
    --__git_log_shortlog_options="
    --	--author= --committer= --grep=
    --	--all-match --invert-grep
    --"
    --# Options accepted by log and show
    --__git_log_show_options="
    --	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
    --"
    --
    --__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
    --
    --__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:"
    --
    --
    --# 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.
    --__git_complete_log_opts ()
    --{
    --	[ -z "$COMPREPLY" ] || return 1   # Precondition
    --
    --	local merge=""
    --	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
    --		merge="--merge"
    --	fi
    --	case "$prev,$cur" in
    --	-L,:*:*)
    --		return	# fall back to Bash filename completion
    --		;;
    --	-L,:*)
    --		__git_complete_symbol --cur="${cur#:}" --sfx=":"
    --		return
    --		;;
    --	-G,*|-S,*)
    --		__git_complete_symbol
    --		return
    --		;;
    --	esac
    --	case "$cur" in
    --	--pretty=*|--format=*)
    --		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
    --			" "" "${cur#*=}"
    --		return
    --		;;
    --	--date=*)
    --		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
    --		return
    --		;;
    --	--decorate=*)
    --		__gitcomp "full short no" "" "${cur##--decorate=}"
    --		return
    --		;;
    --	--diff-algorithm=*)
    --		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
    --		return
    --		;;
    --	--submodule=*)
    --		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
    --		return
    --		;;
    --	--ws-error-highlight=*)
    --		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
    --		return
    --		;;
    --	--no-walk=*)
    --		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
    --		return
    --		;;
    --	--diff-merges=*)
    --                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
    --                return
    --                ;;
    --	--*)
    --		__gitcomp "
    --			$__git_log_common_options
    --			$__git_log_shortlog_options
    --			$__git_log_gitk_options
    --			$__git_log_show_options
    --			--root --topo-order --date-order --reverse
    --			--follow --full-diff
    --			--abbrev-commit --no-abbrev-commit --abbrev=
    --			--relative-date --date=
    --			--pretty= --format= --oneline
    --			--show-signature
    --			--cherry-mark
    --			--cherry-pick
    --			--graph
    --			--decorate --decorate= --no-decorate
    --			--walk-reflogs
    --			--no-walk --no-walk= --do-walk
    --			--parents --children
    --			--expand-tabs --expand-tabs= --no-expand-tabs
    --			$merge
    --			$__git_diff_common_options
    --			"
    --		return
    --		;;
    --	-L:*:*)
    --		return	# fall back to Bash filename completion
    --		;;
    --	-L:*)
    --		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
    --		return
    --		;;
    --	-G*)
    --		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
    --		return
    --		;;
    --	-S*)
    --		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
    --		return
    --		;;
    --	esac
    --}
    --
      _git_log ()
      {
      	__git_has_doubledash && return
4:  c5bee633b2 < -:  ---------- completion: custom git-bisect terms
5:  2bd0cb26f1 < -:  ---------- completion: git-bisect view recognized but not completed
-:  ---------- > 5:  733613d1ed completion: log: use __git_complete_log_opts
-:  ---------- > 6:  06f5973b3b completion: bisect: complete log opts for visualize subcommand
-:  ---------- > 7:  1dc9323f24 completion: bisect: recognize but do not complete view subcommand
-:  ---------- > 8:  451b7a4467 completion: add tests for git-bisect
--
2.43.0

[PATCH v4 1/8] completion: bisect: complete bad, new, old, and help subcommands

[Britton Leo Kerin]

The bad, new, old and help subcommands to git-bisect(1) are not
completed.

Add the bad, new, old, and help subcommands to the appropriate lists
such that the commands and their possible ref arguments are completed.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..06d0b156e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad good skip reset visualize replay log run"
+	local subcommands="start bad new good old skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1462,7 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|good|reset|skip|start)
+	bad|new|good|old|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
-- 
2.43.0

Re: [PATCH v4 1/8] completion: bisect: complete bad, new, old, and help subcommands

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

On Sun, Jan 28, 2024 at 01:34:40PM -0900, Britton Leo Kerin wrote:
> The bad, new, old and help subcommands to git-bisect(1) are not
> completed.
> 
> Add the bad, new, old, and help subcommands to the appropriate lists
> such that the commands and their possible ref arguments are completed.
> 
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 185b47d802..06d0b156e7 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1449,7 +1449,7 @@ _git_bisect ()
>  {
>  	__git_has_doubledash && return
>  
> -	local subcommands="start bad good skip reset visualize replay log run"
> +	local subcommands="start bad new good old skip reset visualize replay log run help"
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
>  	if [ -z "$subcommand" ]; then
>  		__git_find_repo_path
> @@ -1462,7 +1462,7 @@ _git_bisect ()
>  	fi
>  
>  	case "$subcommand" in
> -	bad|good|reset|skip|start)
> +	bad|new|good|old|reset|skip|start)
>  		__git_complete_refs
>  		;;
>  	*)

I didn't even know that `git bisect reset` takes a commit :)

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 2/8] completion: bisect: complete custom terms and related options

[Britton Leo Kerin]

git bisect supports the use of custom terms via the --term-(new|bad) and
--term-(old|good) options, but the completion code doesn't know about
these options or the new subcommands they define.

Add support for these options and the custom subcommands by checking for
them if a bisection is in progress and adding them to the list of
subcommands.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 06d0b156e7..8baf330824 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,20 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad new good old skip reset visualize replay log run help"
+	__git_find_repo_path
+
+	# If a bisection is in progress get the terms being used.
+	local term_bad term_good
+	if [ -f "$__git_repo_path"/BISECT_START ]; then
+		term_bad=$(__git bisect terms --term-bad)
+		term_good=$(__git bisect terms --term-good)
+	fi
+
+	# 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.
+	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1475,22 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|new|good|old|reset|skip|start)
+	start)
+		case "$cur" in
+		--*)
+			__gitcomp "--term-new --term-bad --term-old --term-good"
+			return
+			;;
+		*)
+			__git_complete_refs
+			;;
+		esac
+		;;
+	terms)
+		__gitcomp "--term-good --term-old --term-bad --term-new"
+		return
+		;;
+	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
 		__git_complete_refs
 		;;
 	*)
-- 
2.43.0

Re: [PATCH v4 2/8] completion: bisect: complete custom terms and related options

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]

On Sun, Jan 28, 2024 at 01:34:41PM -0900, Britton Leo Kerin wrote:
> git bisect supports the use of custom terms via the --term-(new|bad) and
> --term-(old|good) options, but the completion code doesn't know about
> these options or the new subcommands they define.
> 
> Add support for these options and the custom subcommands by checking for
> them if a bisection is in progress and adding them to the list of
> subcommands.
> 
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 06d0b156e7..8baf330824 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1449,7 +1449,20 @@ _git_bisect ()
>  {
>  	__git_has_doubledash && return
>  
> -	local subcommands="start bad new good old skip reset visualize replay log run help"
> +	__git_find_repo_path
> +
> +	# If a bisection is in progress get the terms being used.
> +	local term_bad term_good
> +	if [ -f "$__git_repo_path"/BISECT_START ]; then
> +		term_bad=$(__git bisect terms --term-bad)
> +		term_good=$(__git bisect terms --term-good)
> +	fi

Nit: instead of checking for `BISECT_START` we should rather check for
`BISECT_TERMS`. Like that we don't waste two processes for users who
didn't specify any terms, which should be the majority.

We could also parse the terms directly from the file... but I'm not sure
that is really worth it and feels a lot more fragile to me. So I'm not
sure whether or not that is a good idea.

Patrick

> +	# 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.
> +	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
>  	if [ -z "$subcommand" ]; then
>  		__git_find_repo_path
> @@ -1462,7 +1475,22 @@ _git_bisect ()
>  	fi
>  
>  	case "$subcommand" in
> -	bad|new|good|old|reset|skip|start)
> +	start)
> +		case "$cur" in
> +		--*)
> +			__gitcomp "--term-new --term-bad --term-old --term-good"
> +			return
> +			;;
> +		*)
> +			__git_complete_refs
> +			;;
> +		esac
> +		;;
> +	terms)
> +		__gitcomp "--term-good --term-old --term-bad --term-new"
> +		return
> +		;;
> +	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
>  		__git_complete_refs
>  		;;
>  	*)
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 3/8] completion: bisect: complete missing --first-parent and --no-checkout options

[Britton Leo Kerin]

The --first-parent and --no-checkout options to the start subcommand of
git-bisect(1) are not completed.

Enable completion of the --first-parent and --no-checkout options to the
start subcommand.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8baf330824..2ed600244a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1478,7 +1478,7 @@ _git_bisect ()
 	start)
 		case "$cur" in
 		--*)
-			__gitcomp "--term-new --term-bad --term-old --term-good"
+			__gitcomp "--first-parent --no-checkout --term-new --term-bad --term-old --term-good"
 			return
 			;;
 		*)
-- 
2.43.0

[PATCH v4 4/8] completion: new function __git_complete_log_opts

[Britton Leo Kerin]

The options accepted by git-log are also accepted by at least one other
command (git-bisect).  Prepare to factor out the common option and
option argument completion code by defining a new function.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 101 +++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2ed600244a..dfd504c37e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2089,6 +2089,107 @@ __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:"
 
+# Complete porcelain (i.e. not git-rev-list) options and at least some
+# option arguments accepted by git-log.  Note that this same set of options
+# are also accepted by some other git commands besides git-log.
+__git_complete_log_opts ()
+{
+        COMPREPLY=""
+
+	local merge=""
+	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
+		merge="--merge"
+	fi
+	case "$prev,$cur" in
+	-L,:*:*)
+		return	# fall back to Bash filename completion
+		;;
+	-L,:*)
+		__git_complete_symbol --cur="${cur#:}" --sfx=":"
+		return
+		;;
+	-G,*|-S,*)
+		__git_complete_symbol
+		return
+		;;
+	esac
+	case "$cur" in
+	--pretty=*|--format=*)
+		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
+			" "" "${cur#*=}"
+		return
+		;;
+	--date=*)
+		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
+		return
+		;;
+	--decorate=*)
+		__gitcomp "full short no" "" "${cur##--decorate=}"
+		return
+		;;
+	--diff-algorithm=*)
+		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+		return
+		;;
+	--submodule=*)
+		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
+		return
+		;;
+	--ws-error-highlight=*)
+		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
+		return
+		;;
+	--no-walk=*)
+		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
+		return
+		;;
+	--diff-merges=*)
+                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
+                return
+                ;;
+	--*)
+		__gitcomp "
+			$__git_log_common_options
+			$__git_log_shortlog_options
+			$__git_log_gitk_options
+			$__git_log_show_options
+			--root --topo-order --date-order --reverse
+			--follow --full-diff
+			--abbrev-commit --no-abbrev-commit --abbrev=
+			--relative-date --date=
+			--pretty= --format= --oneline
+			--show-signature
+			--cherry-mark
+			--cherry-pick
+			--graph
+			--decorate --decorate= --no-decorate
+			--walk-reflogs
+			--no-walk --no-walk= --do-walk
+			--parents --children
+			--expand-tabs --expand-tabs= --no-expand-tabs
+			$merge
+			$__git_diff_common_options
+			"
+		return
+		;;
+	-L:*:*)
+		return	# fall back to Bash filename completion
+		;;
+	-L:*)
+		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
+		return
+		;;
+	-G*)
+		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
+		return
+		;;
+	-S*)
+		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
+		return
+		;;
+	esac
+}
+
 _git_log ()
 {
 	__git_has_doubledash && return
-- 
2.43.0

[PATCH v4 5/8] completion: log: use __git_complete_log_opts

[Britton Leo Kerin]

Use the new __git_complete_log_opts function to handle option and
optiona rgument completion in _git_log.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 95 +-------------------------
 1 file changed, 3 insertions(+), 92 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dfd504c37e..41c76c1246 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2195,98 +2195,9 @@ _git_log ()
 	__git_has_doubledash && return
 	__git_find_repo_path
 
-	local merge=""
-	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
-		merge="--merge"
-	fi
-	case "$prev,$cur" in
-	-L,:*:*)
-		return	# fall back to Bash filename completion
-		;;
-	-L,:*)
-		__git_complete_symbol --cur="${cur#:}" --sfx=":"
-		return
-		;;
-	-G,*|-S,*)
-		__git_complete_symbol
-		return
-		;;
-	esac
-	case "$cur" in
-	--pretty=*|--format=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur#*=}"
-		return
-		;;
-	--date=*)
-		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
-		return
-		;;
-	--decorate=*)
-		__gitcomp "full short no" "" "${cur##--decorate=}"
-		return
-		;;
-	--diff-algorithm=*)
-		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
-		return
-		;;
-	--submodule=*)
-		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
-		return
-		;;
-	--ws-error-highlight=*)
-		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
-		return
-		;;
-	--no-walk=*)
-		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
-		return
-		;;
-	--diff-merges=*)
-                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
-                return
-                ;;
-	--*)
-		__gitcomp "
-			$__git_log_common_options
-			$__git_log_shortlog_options
-			$__git_log_gitk_options
-			$__git_log_show_options
-			--root --topo-order --date-order --reverse
-			--follow --full-diff
-			--abbrev-commit --no-abbrev-commit --abbrev=
-			--relative-date --date=
-			--pretty= --format= --oneline
-			--show-signature
-			--cherry-mark
-			--cherry-pick
-			--graph
-			--decorate --decorate= --no-decorate
-			--walk-reflogs
-			--no-walk --no-walk= --do-walk
-			--parents --children
-			--expand-tabs --expand-tabs= --no-expand-tabs
-			$merge
-			$__git_diff_common_options
-			"
-		return
-		;;
-	-L:*:*)
-		return	# fall back to Bash filename completion
-		;;
-	-L:*)
-		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
-		return
-		;;
-	-G*)
-		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
-		return
-		;;
-	-S*)
-		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
-		return
-		;;
-	esac
+	__git_complete_log_opts
+	[ -z "$COMPREPLY" ] || return
+
 	__git_complete_revlist
 }
 
-- 
2.43.0

Re: [PATCH v4 5/8] completion: log: use __git_complete_log_opts

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3446 bytes --]

On Sun, Jan 28, 2024 at 01:34:44PM -0900, Britton Leo Kerin wrote:
> Use the new __git_complete_log_opts function to handle option and
> optiona rgument completion in _git_log.

I think this commit could be merged with the preceding one to clarify
that this really only is a move of code. Sorry if my comments on the
previous round weren't clear on that.

Patrick

> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 95 +-------------------------
>  1 file changed, 3 insertions(+), 92 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index dfd504c37e..41c76c1246 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2195,98 +2195,9 @@ _git_log ()
>  	__git_has_doubledash && return
>  	__git_find_repo_path
>  
> -	local merge=""
> -	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> -		merge="--merge"
> -	fi
> -	case "$prev,$cur" in
> -	-L,:*:*)
> -		return	# fall back to Bash filename completion
> -		;;
> -	-L,:*)
> -		__git_complete_symbol --cur="${cur#:}" --sfx=":"
> -		return
> -		;;
> -	-G,*|-S,*)
> -		__git_complete_symbol
> -		return
> -		;;
> -	esac
> -	case "$cur" in
> -	--pretty=*|--format=*)
> -		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
> -			" "" "${cur#*=}"
> -		return
> -		;;
> -	--date=*)
> -		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
> -		return
> -		;;
> -	--decorate=*)
> -		__gitcomp "full short no" "" "${cur##--decorate=}"
> -		return
> -		;;
> -	--diff-algorithm=*)
> -		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
> -		return
> -		;;
> -	--submodule=*)
> -		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
> -		return
> -		;;
> -	--ws-error-highlight=*)
> -		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
> -		return
> -		;;
> -	--no-walk=*)
> -		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
> -		return
> -		;;
> -	--diff-merges=*)
> -                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
> -                return
> -                ;;
> -	--*)
> -		__gitcomp "
> -			$__git_log_common_options
> -			$__git_log_shortlog_options
> -			$__git_log_gitk_options
> -			$__git_log_show_options
> -			--root --topo-order --date-order --reverse
> -			--follow --full-diff
> -			--abbrev-commit --no-abbrev-commit --abbrev=
> -			--relative-date --date=
> -			--pretty= --format= --oneline
> -			--show-signature
> -			--cherry-mark
> -			--cherry-pick
> -			--graph
> -			--decorate --decorate= --no-decorate
> -			--walk-reflogs
> -			--no-walk --no-walk= --do-walk
> -			--parents --children
> -			--expand-tabs --expand-tabs= --no-expand-tabs
> -			$merge
> -			$__git_diff_common_options
> -			"
> -		return
> -		;;
> -	-L:*:*)
> -		return	# fall back to Bash filename completion
> -		;;
> -	-L:*)
> -		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
> -		return
> -		;;
> -	-G*)
> -		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
> -		return
> -		;;
> -	-S*)
> -		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
> -		return
> -		;;
> -	esac
> +	__git_complete_log_opts
> +	[ -z "$COMPREPLY" ] || return
> +
>  	__git_complete_revlist
>  }
>  
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 6/8] completion: bisect: complete log opts for visualize subcommand

[Britton Leo Kerin]

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.

Make completion of porcelain git-log options and option arguments to the
visualize subcommand work by calling __git_complete_log_opts when the
start of an option to the subcommand is seen (visualize doesn't support
any options besides the git-log options).

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 41c76c1246..ae16e742a4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1490,6 +1490,10 @@ _git_bisect ()
 		__gitcomp "--term-good --term-old --term-bad --term-new"
 		return
 		;;
+	visualize)
+		__git_complete_log_opts
+		return
+		;;
 	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
 		__git_complete_refs
 		;;
-- 
2.43.0

[PATCH v4 7/8] completion: bisect: recognize but do not complete view subcommand

[Britton Leo Kerin]

The "view" alias for the visualize subcommand is neither completed nor
recognized.  It's undesirable to complete it because it's first letters
are the same as for visualize, making completion less rather than more
efficient without adding much in the way of interface discovery.
However, it needs to be recognized in order to enable log option
completion for it.

Recognize but do not complete the view command by creating and using
separate lists of completable_subcommands and all_subcommands.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ae16e742a4..0cf1a5a393 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1462,12 +1462,19 @@ _git_bisect ()
 	# 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.
-	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
-	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	#
+	# 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 "$all_subcommands")"
+
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
 		if [ -f "$__git_repo_path"/BISECT_START ]; then
-			__gitcomp "$subcommands"
+			__gitcomp "$completable_subcommands"
 		else
 			__gitcomp "replay start"
 		fi
@@ -1490,7 +1497,7 @@ _git_bisect ()
 		__gitcomp "--term-good --term-old --term-bad --term-new"
 		return
 		;;
-	visualize)
+	visualize|view)
 		__git_complete_log_opts
 		return
 		;;
-- 
2.43.0

[PATCH v4 8/8] completion: add tests for git-bisect

[Britton Leo Kerin]

There aren't any tests for completion of git bisect and it's
subcommands.

Add tests.
---
 t/t9902-completion.sh | 135 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..698e278450 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1259,6 +1259,141 @@ test_expect_success 'git switch - with no options, complete local branches and u
 	EOF
 '
 
+test_expect_success 'git bisect - when not bisecting, complete only replay and start subcommands' '
+	test_completion "git bisect " <<-\EOF
+	replay Z
+	start Z
+	EOF
+'
+
+test_expect_success 'git bisect - complete options to start subcommand' '
+	test_completion "git bisect start --" <<-\EOF
+	--term-new Z
+	--term-bad Z
+	--term-old Z
+	--term-good Z
+	--no-checkout Z
+	--first-parent Z
+	EOF
+'
+
+test_expect_success 'setup for git-bisect tests requiring a repo' '
+	git init git-bisect &&
+	(
+		cd git-bisect &&
+		echo "initial contents" >file &&
+		git add file &&
+		git commit -am "Initial commit" &&
+		git tag initial &&
+		echo "new line" >>file &&
+		git commit -am "First change" &&
+		echo "another new line" >>file &&
+		git commit -am "Second change" &&
+		git tag final
+	)
+'
+
+test_expect_success 'git bisect - start subcommand arguments before double-dash are completed as revs' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect start " <<-\EOF
+		HEAD Z
+		final Z
+		initial Z
+		master Z
+		EOF
+	)
+'
+
+# Note that these arguments are <pathspec>s, which in practice the fallback
+# completion (not the git completion) later ends up completing as paths.
+test_expect_success 'git bisect - start subcommand arguments after double-dash are not completed' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect start final initial -- " ""
+	)
+'
+
+test_expect_success 'setup for git-bisect tests requiring ongoing bisection' '
+	(
+		cd git-bisect &&
+		git bisect start --term-new=custom_new --term-old=custom_old final initial
+	)
+'
+
+test_expect_success 'git-bisect - when bisecting all subcommands are candidates' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect " <<-\EOF
+		start Z
+		bad Z
+		custom_new Z
+		custom_old Z
+		new Z
+		good Z
+		old Z
+		terms Z
+		skip Z
+		reset Z
+		visualize Z
+		replay Z
+		log Z
+		run Z
+		help Z
+		EOF
+	)
+'
+test_expect_success 'git-bisect - options to terms subcommand are candidates' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect terms --" <<-\EOF
+		--term-bad Z
+		--term-good Z
+		--term-new Z
+		--term-old Z
+		EOF
+	)
+'
+
+test_expect_success 'git-bisect - git-log options to visualize subcommand are candidates' '
+	(
+		cd git-bisect &&
+		# The completion used for git-log and here does not complete
+		# every git-log option, so rather than hope to stay in sync
+		# with exactly what it does we will just spot-test here.
+		test_completion "git bisect visualize --sta" <<-\EOF &&
+		--stat Z
+		EOF
+		test_completion "git bisect visualize --summar" <<-\EOF
+		--summary Z
+		EOF
+	)
+'
+
+test_expect_success 'git-bisect - view subcommand is not a candidate' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect vi" <<-\EOF
+		visualize Z
+		EOF
+	)
+'
+
+test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
+	(
+		cd git-bisect &&
+		# The completion used for git-log and here does not complete
+		# every git-log option, so rather than hope to stay in sync
+		# with exactly what it does we will just spot-test here.
+		test_completion "git bisect view --sta" <<-\EOF &&
+		--stat Z
+		EOF
+		test_completion "git bisect view --summar" <<-\EOF
+		--summary Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

Re: [PATCH v4 8/8] completion: add tests for git-bisect

[Junio C Hamano]

Britton Leo Kerin <britton.kerin@gmail.com> writes:

> +test_expect_success 'git bisect - start subcommand arguments before double-dash are completed as revs' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect start " <<-\EOF
> +		HEAD Z
> +		final Z
> +		initial Z
> +		master Z
> +		EOF
> +	)
> +'

When GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME is set to 'main' (can be
seen as a failure in linux-gcc job in GitHub CI), this piece breaks
the test, because 'master' would not appear there in the list.

You could detect what the initial default branch name currently is
and use that branch name to dynamically generate the above list
during the test.  I do not think it is worth it, and forcing the
fixed name should be sufficient.

Perhaps like this (not tested):

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 698e278450..26f616fcfe 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -5,6 +5,8 @@
 
 test_description='test bash completion'
 
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./lib-bash.sh
 
 complete ()

Re: [PATCH v4 8/8] completion: add tests for git-bisect

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4911 bytes --]

On Sun, Jan 28, 2024 at 01:34:47PM -0900, Britton Leo Kerin wrote:
> There aren't any tests for completion of git bisect and it's
> subcommands.
> 
> Add tests.

I think it would be nice if you added relevant tests directly in the
commits that introduce the new completions. E.g. add a test for the
bisect terms in the same commit where you introduce the completion for
it.

Like that you can easily add tests one by one, which decreases the
review load. Also, it serves to demonstrate both that the functionality
works and helps the reviewer to understand better what exactly you are
adding by having a nice example.

Patrick

> ---
>  t/t9902-completion.sh | 135 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index aa9a614de3..698e278450 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1259,6 +1259,141 @@ test_expect_success 'git switch - with no options, complete local branches and u
>  	EOF
>  '
>  
> +test_expect_success 'git bisect - when not bisecting, complete only replay and start subcommands' '
> +	test_completion "git bisect " <<-\EOF
> +	replay Z
> +	start Z
> +	EOF
> +'
> +
> +test_expect_success 'git bisect - complete options to start subcommand' '
> +	test_completion "git bisect start --" <<-\EOF
> +	--term-new Z
> +	--term-bad Z
> +	--term-old Z
> +	--term-good Z
> +	--no-checkout Z
> +	--first-parent Z
> +	EOF
> +'
> +
> +test_expect_success 'setup for git-bisect tests requiring a repo' '
> +	git init git-bisect &&
> +	(
> +		cd git-bisect &&
> +		echo "initial contents" >file &&
> +		git add file &&
> +		git commit -am "Initial commit" &&
> +		git tag initial &&
> +		echo "new line" >>file &&
> +		git commit -am "First change" &&
> +		echo "another new line" >>file &&
> +		git commit -am "Second change" &&
> +		git tag final
> +	)
> +'
> +
> +test_expect_success 'git bisect - start subcommand arguments before double-dash are completed as revs' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect start " <<-\EOF
> +		HEAD Z
> +		final Z
> +		initial Z
> +		master Z
> +		EOF
> +	)
> +'
> +
> +# Note that these arguments are <pathspec>s, which in practice the fallback
> +# completion (not the git completion) later ends up completing as paths.
> +test_expect_success 'git bisect - start subcommand arguments after double-dash are not completed' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect start final initial -- " ""
> +	)
> +'
> +
> +test_expect_success 'setup for git-bisect tests requiring ongoing bisection' '
> +	(
> +		cd git-bisect &&
> +		git bisect start --term-new=custom_new --term-old=custom_old final initial
> +	)
> +'
> +
> +test_expect_success 'git-bisect - when bisecting all subcommands are candidates' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect " <<-\EOF
> +		start Z
> +		bad Z
> +		custom_new Z
> +		custom_old Z
> +		new Z
> +		good Z
> +		old Z
> +		terms Z
> +		skip Z
> +		reset Z
> +		visualize Z
> +		replay Z
> +		log Z
> +		run Z
> +		help Z
> +		EOF
> +	)
> +'
> +test_expect_success 'git-bisect - options to terms subcommand are candidates' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect terms --" <<-\EOF
> +		--term-bad Z
> +		--term-good Z
> +		--term-new Z
> +		--term-old Z
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'git-bisect - git-log options to visualize subcommand are candidates' '
> +	(
> +		cd git-bisect &&
> +		# The completion used for git-log and here does not complete
> +		# every git-log option, so rather than hope to stay in sync
> +		# with exactly what it does we will just spot-test here.
> +		test_completion "git bisect visualize --sta" <<-\EOF &&
> +		--stat Z
> +		EOF
> +		test_completion "git bisect visualize --summar" <<-\EOF
> +		--summary Z
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'git-bisect - view subcommand is not a candidate' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect vi" <<-\EOF
> +		visualize Z
> +		EOF
> +	)
> +'
> +
> +test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
> +	(
> +		cd git-bisect &&
> +		# The completion used for git-log and here does not complete
> +		# every git-log option, so rather than hope to stay in sync
> +		# with exactly what it does we will just spot-test here.
> +		test_completion "git bisect view --sta" <<-\EOF &&
> +		--stat Z
> +		EOF
> +		test_completion "git bisect view --summar" <<-\EOF
> +		--summary Z
> +		EOF
> +	)
> +'
> +
>  test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
>  	test_completion "git checkout " <<-\EOF
>  	HEAD Z
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 0/8] completion: improvements for git-bisect

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

On Sun, Jan 28, 2024 at 01:34:39PM -0900, Britton Leo Kerin wrote:
> Relative to v3 this reworks the commit contents and descriptions
> according to review suggestions, removes unnecessary case statements and
> precondition, adds option completion for the terms subcommand, and adds
> tests.

Thanks for this new version! I've got a few more comments, but overall I
really like what I see here and think that this is getting close.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 0/7] completion: improvements for git-bisect

[Britton Leo Kerin]

Relative to v4 this make the following actual changes:

  * fixes GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME to 'master' for all of
    t9902-completion.sh as suggested by Junio.  This change affects all
    of t9902-completion.sh so I've put it by itself in it's own commit.

  * uses BISECT_TERMS to avoid pointless processes as suggested by Patrick.

The commits are also refactored as follows:

  * squashes the introduction of __git_complete_log_opts in with it's
    first use a suggested by Patrick.

  * spreads tests across commits as suggest by Patrick.

Thanks for the reviews.

Britton Leo Kerin (7):
  completion: tests: always use 'master' for default initial branch name
  completion: bisect: complete bad, new, old, and help subcommands
  completion: bisect: complete custom terms and related options
  completion: bisect: complete missing --first-parent and --no-checkout
    options
  completion: new function __git_complete_log_opts
  completion: bisect: complete log opts for visualize subcommand
  completion: bisect: recognize but do not complete view subcommand

 contrib/completion/git-completion.bash |  65 ++++++++++--
 t/t9902-completion.sh                  | 140 +++++++++++++++++++++++++
 2 files changed, 198 insertions(+), 7 deletions(-)

Range-diff against v4:
1:  66153024c3 < -:  ---------- completion: bisect: complete bad, new, old, and help subcommands
-:  ---------- > 1:  71b73de914 completion: tests: always use 'master' for default initial branch name
8:  451b7a4467 ! 2:  3a478a7a08 completion: add tests for git-bisect
    @@ Metadata
     Author: Britton Leo Kerin <britton.kerin@gmail.com>

      ## Commit message ##
    -    completion: add tests for git-bisect
    +    completion: bisect: complete bad, new, old, and help subcommands

    -    There aren't any tests for completion of git bisect and it's
    -    subcommands.
    +    The bad, new, old and help subcommands to git-bisect(1) are not
    +    completed.

    +    Add the bad, new, old, and help subcommands to the appropriate lists
    +    such that the commands and their possible ref arguments are completed.
         Add tests.

    +    Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.c
    +
    + ## contrib/completion/git-completion.bash ##
    +@@ contrib/completion/git-completion.bash: _git_bisect ()
    + {
    + 	__git_has_doubledash && return
    +
    +-	local subcommands="start bad good skip reset visualize replay log run"
    ++	local subcommands="start bad new good old skip reset visualize replay log run help"
    + 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
    + 	if [ -z "$subcommand" ]; then
    + 		__git_find_repo_path
    +@@ contrib/completion/git-completion.bash: _git_bisect ()
    + 	fi
    +
    + 	case "$subcommand" in
    +-	bad|good|reset|skip|start)
    ++	bad|new|good|old|reset|skip|start)
    + 		__git_complete_refs
    + 		;;
    + 	*)
    +
      ## t/t9902-completion.sh ##
     @@ t/t9902-completion.sh: test_expect_success 'git switch - with no options, complete local branches and u
      	EOF
    @@ t/t9902-completion.sh: test_expect_success 'git switch - with no options, comple
     +	EOF
     +'
     +
    -+test_expect_success 'git bisect - complete options to start subcommand' '
    -+	test_completion "git bisect start --" <<-\EOF
    -+	--term-new Z
    -+	--term-bad Z
    -+	--term-old Z
    -+	--term-good Z
    -+	--no-checkout Z
    -+	--first-parent Z
    -+	EOF
    -+'
    -+
     +test_expect_success 'setup for git-bisect tests requiring a repo' '
     +	git init git-bisect &&
     +	(
    @@ t/t9902-completion.sh: test_expect_success 'git switch - with no options, comple
     +		test_completion "git bisect " <<-\EOF
     +		start Z
     +		bad Z
    -+		custom_new Z
    -+		custom_old Z
     +		new Z
     +		good Z
     +		old Z
    -+		terms Z
     +		skip Z
     +		reset Z
     +		visualize Z
    @@ t/t9902-completion.sh: test_expect_success 'git switch - with no options, comple
     +		EOF
     +	)
     +'
    -+test_expect_success 'git-bisect - options to terms subcommand are candidates' '
    -+	(
    -+		cd git-bisect &&
    -+		test_completion "git bisect terms --" <<-\EOF
    -+		--term-bad Z
    -+		--term-good Z
    -+		--term-new Z
    -+		--term-old Z
    -+		EOF
    -+	)
    -+'
    -+
    -+test_expect_success 'git-bisect - git-log options to visualize subcommand are candidates' '
    -+	(
    -+		cd git-bisect &&
    -+		# The completion used for git-log and here does not complete
    -+		# every git-log option, so rather than hope to stay in sync
    -+		# with exactly what it does we will just spot-test here.
    -+		test_completion "git bisect visualize --sta" <<-\EOF &&
    -+		--stat Z
    -+		EOF
    -+		test_completion "git bisect visualize --summar" <<-\EOF
    -+		--summary Z
    -+		EOF
    -+	)
    -+'
    -+
    -+test_expect_success 'git-bisect - view subcommand is not a candidate' '
    -+	(
    -+		cd git-bisect &&
    -+		test_completion "git bisect vi" <<-\EOF
    -+		visualize Z
    -+		EOF
    -+	)
    -+'
    -+
    -+test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
    -+	(
    -+		cd git-bisect &&
    -+		# The completion used for git-log and here does not complete
    -+		# every git-log option, so rather than hope to stay in sync
    -+		# with exactly what it does we will just spot-test here.
    -+		test_completion "git bisect view --sta" <<-\EOF &&
    -+		--stat Z
    -+		EOF
    -+		test_completion "git bisect view --summar" <<-\EOF
    -+		--summary Z
    -+		EOF
    -+	)
    -+'
     +
      test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
      	test_completion "git checkout " <<-\EOF
2:  7eb8c842a3 ! 3:  fab7159cf4 completion: bisect: complete custom terms and related options
    @@ Commit message
         these options or the new subcommands they define.

         Add support for these options and the custom subcommands by checking for
    -    them if a bisection is in progress and adding them to the list of
    -    subcommands.
    +    BISECT_TERMS and adding them to the list of subcommands.  Add tests.

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

    @@ contrib/completion/git-completion.bash: _git_bisect ()
     +
     +	# If a bisection is in progress get the terms being used.
     +	local term_bad term_good
    -+	if [ -f "$__git_repo_path"/BISECT_START ]; then
    ++	if [ -f "$__git_repo_path"/BISECT_TERMS ]; then
     +		term_bad=$(__git bisect terms --term-bad)
     +		term_good=$(__git bisect terms --term-good)
     +	fi
    @@ contrib/completion/git-completion.bash: _git_bisect ()
      		__git_complete_refs
      		;;
      	*)
    +
    + ## t/t9902-completion.sh ##
    +@@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
    + 		test_completion "git bisect " <<-\EOF
    + 		start Z
    + 		bad Z
    ++		custom_new Z
    ++		custom_old Z
    + 		new Z
    + 		good Z
    + 		old Z
    ++		terms Z
    + 		skip Z
    + 		reset Z
    + 		visualize Z
    +@@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
    + 		EOF
    + 	)
    + '
    ++test_expect_success 'git-bisect - options to terms subcommand are candidates' '
    ++	(
    ++		cd git-bisect &&
    ++		test_completion "git bisect terms --" <<-\EOF
    ++		--term-bad Z
    ++		--term-good Z
    ++		--term-new Z
    ++		--term-old Z
    ++		EOF
    ++	)
    ++'
    ++
    +
    + test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
    + 	test_completion "git checkout " <<-\EOF
3:  5f5076bb93 < -:  ---------- completion: bisect: complete missing --first-parent and --no-checkout options
4:  c8ffa0e915 < -:  ---------- completion: new function __git_complete_log_opts
5:  733613d1ed < -:  ---------- completion: log: use __git_complete_log_opts
-:  ---------- > 4:  73f3343b94 completion: bisect: complete missing --first-parent and --no-checkout options
-:  ---------- > 5:  a20846bbd3 completion: new function __git_complete_log_opts
6:  06f5973b3b ! 6:  fe5545c9a3 completion: bisect: complete log opts for visualize subcommand
    @@ Commit message
         Make completion of porcelain git-log options and option arguments to the
         visualize subcommand work by calling __git_complete_log_opts when the
         start of an option to the subcommand is seen (visualize doesn't support
    -    any options besides the git-log options).
    +    any options besides the git-log options).  Add test.

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

    @@ contrib/completion/git-completion.bash: _git_bisect ()
      	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
      		__git_complete_refs
      		;;
    +
    + ## t/t9902-completion.sh ##
    +@@ t/t9902-completion.sh: test_expect_success 'git-bisect - options to terms subcommand are candidates' '
    + 	)
    + '
    +
    ++test_expect_success 'git-bisect - git-log options to visualize subcommand are candidates' '
    ++	(
    ++		cd git-bisect &&
    ++		# The completion used for git-log and here does not complete
    ++		# every git-log option, so rather than hope to stay in sync
    ++		# with exactly what it does we will just spot-test here.
    ++		test_completion "git bisect visualize --sta" <<-\EOF &&
    ++		--stat Z
    ++		EOF
    ++		test_completion "git bisect visualize --summar" <<-\EOF
    ++		--summary Z
    ++		EOF
    ++	)
    ++'
    +
    + test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
    + 	test_completion "git checkout " <<-\EOF
7:  1dc9323f24 ! 7:  c9102ac532 completion: bisect: recognize but do not complete view subcommand
    @@ Commit message
         completion for it.

         Recognize but do not complete the view command by creating and using
    -    separate lists of completable_subcommands and all_subcommands.
    +    separate lists of completable_subcommands and all_subcommands.  Add
    +    tests.

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

    @@ contrib/completion/git-completion.bash: _git_bisect ()
      		__git_complete_log_opts
      		return
      		;;
    +
    + ## t/t9902-completion.sh ##
    +@@ t/t9902-completion.sh: test_expect_success 'git-bisect - git-log options to visualize subcommand are ca
    + 	)
    + '
    +
    ++test_expect_success 'git-bisect - view subcommand is not a candidate' '
    ++	(
    ++		cd git-bisect &&
    ++		test_completion "git bisect vi" <<-\EOF
    ++		visualize Z
    ++		EOF
    ++	)
    ++'
    ++
    ++test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
    ++	(
    ++		cd git-bisect &&
    ++		# The completion used for git-log and here does not complete
    ++		# every git-log option, so rather than hope to stay in sync
    ++		# with exactly what it does we will just spot-test here.
    ++		test_completion "git bisect view --sta" <<-\EOF &&
    ++		--stat Z
    ++		EOF
    ++		test_completion "git bisect view --summar" <<-\EOF
    ++		--summary Z
    ++		EOF
    ++	)
    ++'
    ++
    + test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
    + 	test_completion "git checkout " <<-\EOF
    + 	HEAD Z
--
2.43.0

[PATCH v5 1/7] completion: tests: always use 'master' for default initial branch name

[Britton Leo Kerin]

The default initial branch name can normally be configured using the
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME environment variable.  However,
when testing e.g. <rev> completion it's convenient to know the
exact initial branch name that will be used.

To achieve that without too much trouble it is considered sufficient
to force the default initial branch name to 'master' for all of
t9902-completion.sh.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 t/t9902-completion.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..a5d4e900a2 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -5,6 +5,11 @@
 
 test_description='test bash completion'
 
+# Override environment and always use master for the default initial branch
+# name for these tests, so that rev completion candidates are as expected.
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
 . ./lib-bash.sh
 
 complete ()
-- 
2.43.0

[PATCH v5 2/7] completion: bisect: complete bad, new, old, and help subcommands

[Britton Leo Kerin]

The bad, new, old and help subcommands to git-bisect(1) are not
completed.

Add the bad, new, old, and help subcommands to the appropriate lists
such that the commands and their possible ref arguments are completed.
Add tests.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.c
---
 contrib/completion/git-completion.bash |  4 +-
 t/t9902-completion.sh                  | 71 ++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..06d0b156e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad good skip reset visualize replay log run"
+	local subcommands="start bad new good old skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1462,7 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|good|reset|skip|start)
+	bad|new|good|old|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a5d4e900a2..7388c892cf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1264,6 +1264,77 @@ test_expect_success 'git switch - with no options, complete local branches and u
 	EOF
 '
 
+test_expect_success 'git bisect - when not bisecting, complete only replay and start subcommands' '
+	test_completion "git bisect " <<-\EOF
+	replay Z
+	start Z
+	EOF
+'
+
+test_expect_success 'setup for git-bisect tests requiring a repo' '
+	git init git-bisect &&
+	(
+		cd git-bisect &&
+		echo "initial contents" >file &&
+		git add file &&
+		git commit -am "Initial commit" &&
+		git tag initial &&
+		echo "new line" >>file &&
+		git commit -am "First change" &&
+		echo "another new line" >>file &&
+		git commit -am "Second change" &&
+		git tag final
+	)
+'
+
+test_expect_success 'git bisect - start subcommand arguments before double-dash are completed as revs' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect start " <<-\EOF
+		HEAD Z
+		final Z
+		initial Z
+		master Z
+		EOF
+	)
+'
+
+# Note that these arguments are <pathspec>s, which in practice the fallback
+# completion (not the git completion) later ends up completing as paths.
+test_expect_success 'git bisect - start subcommand arguments after double-dash are not completed' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect start final initial -- " ""
+	)
+'
+
+test_expect_success 'setup for git-bisect tests requiring ongoing bisection' '
+	(
+		cd git-bisect &&
+		git bisect start --term-new=custom_new --term-old=custom_old final initial
+	)
+'
+
+test_expect_success 'git-bisect - when bisecting all subcommands are candidates' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect " <<-\EOF
+		start Z
+		bad Z
+		new Z
+		good Z
+		old Z
+		skip Z
+		reset Z
+		visualize Z
+		replay Z
+		log Z
+		run Z
+		help Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

Re: [PATCH v5 2/7] completion: bisect: complete bad, new, old, and help subcommands

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On Mon, Feb 05, 2024 at 05:09:25PM -0900, Britton Leo Kerin wrote:
> The bad, new, old and help subcommands to git-bisect(1) are not
> completed.
> 
> Add the bad, new, old, and help subcommands to the appropriate lists
> such that the commands and their possible ref arguments are completed.
> Add tests.
> 
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.c

Nit: the SOB has been truncated here. Other than that the patch looks
good to me.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 3/7] completion: bisect: complete custom terms and related options

[Britton Leo Kerin]

git bisect supports the use of custom terms via the --term-(new|bad) and
--term-(old|good) options, but the completion code doesn't know about
these options or the new subcommands they define.

Add support for these options and the custom subcommands by checking for
BISECT_TERMS and adding them to the list of subcommands.  Add tests.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
 t/t9902-completion.sh                  | 15 ++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 06d0b156e7..6a3d9c7760 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,20 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad new good old skip reset visualize replay log run help"
+	__git_find_repo_path
+
+	# If a bisection is in progress get the terms being used.
+	local term_bad term_good
+	if [ -f "$__git_repo_path"/BISECT_TERMS ]; then
+		term_bad=$(__git bisect terms --term-bad)
+		term_good=$(__git bisect terms --term-good)
+	fi
+
+	# 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.
+	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1475,22 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|new|good|old|reset|skip|start)
+	start)
+		case "$cur" in
+		--*)
+			__gitcomp "--term-new --term-bad --term-old --term-good"
+			return
+			;;
+		*)
+			__git_complete_refs
+			;;
+		esac
+		;;
+	terms)
+		__gitcomp "--term-good --term-old --term-bad --term-new"
+		return
+		;;
+	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
 		__git_complete_refs
 		;;
 	*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7388c892cf..409a5a49d5 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1321,9 +1321,12 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
 		test_completion "git bisect " <<-\EOF
 		start Z
 		bad Z
+		custom_new Z
+		custom_old Z
 		new Z
 		good Z
 		old Z
+		terms Z
 		skip Z
 		reset Z
 		visualize Z
@@ -1334,6 +1337,18 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
 		EOF
 	)
 '
+test_expect_success 'git-bisect - options to terms subcommand are candidates' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect terms --" <<-\EOF
+		--term-bad Z
+		--term-good Z
+		--term-new Z
+		--term-old Z
+		EOF
+	)
+'
+
 
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
-- 
2.43.0

Re: [PATCH v5 3/7] completion: bisect: complete custom terms and related options

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3577 bytes --]

On Mon, Feb 05, 2024 at 05:09:26PM -0900, Britton Leo Kerin wrote:
> git bisect supports the use of custom terms via the --term-(new|bad) and
> --term-(old|good) options, but the completion code doesn't know about
> these options or the new subcommands they define.
> 
> Add support for these options and the custom subcommands by checking for
> BISECT_TERMS and adding them to the list of subcommands.  Add tests.
> 
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
>  t/t9902-completion.sh                  | 15 ++++++++++++
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 06d0b156e7..6a3d9c7760 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1449,7 +1449,20 @@ _git_bisect ()
>  {
>  	__git_has_doubledash && return
>  
> -	local subcommands="start bad new good old skip reset visualize replay log run help"
> +	__git_find_repo_path
> +
> +	# If a bisection is in progress get the terms being used.
> +	local term_bad term_good
> +	if [ -f "$__git_repo_path"/BISECT_TERMS ]; then
> +		term_bad=$(__git bisect terms --term-bad)
> +		term_good=$(__git bisect terms --term-good)
> +	fi
> +
> +	# 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.
> +	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"
>  	if [ -z "$subcommand" ]; then
>  		__git_find_repo_path
> @@ -1462,7 +1475,22 @@ _git_bisect ()
>  	fi
>  
>  	case "$subcommand" in
> -	bad|new|good|old|reset|skip|start)
> +	start)
> +		case "$cur" in
> +		--*)
> +			__gitcomp "--term-new --term-bad --term-old --term-good"
> +			return
> +			;;
> +		*)
> +			__git_complete_refs
> +			;;
> +		esac
> +		;;
> +	terms)
> +		__gitcomp "--term-good --term-old --term-bad --term-new"
> +		return
> +		;;
> +	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
>  		__git_complete_refs
>  		;;
>  	*)
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 7388c892cf..409a5a49d5 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1321,9 +1321,12 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
>  		test_completion "git bisect " <<-\EOF
>  		start Z
>  		bad Z
> +		custom_new Z
> +		custom_old Z
>  		new Z
>  		good Z
>  		old Z
> +		terms Z
>  		skip Z
>  		reset Z
>  		visualize Z
> @@ -1334,6 +1337,18 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
>  		EOF
>  	)
>  '

Nit: missing a newline between the tests...

> +test_expect_success 'git-bisect - options to terms subcommand are candidates' '
> +	(
> +		cd git-bisect &&
> +		test_completion "git bisect terms --" <<-\EOF
> +		--term-bad Z
> +		--term-good Z
> +		--term-new Z
> +		--term-old Z
> +		EOF
> +	)
> +'
> +
>  

... wheeras here we have a newline too many now.

Patrick

>  test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
>  	test_completion "git checkout " <<-\EOF
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 4/7] completion: bisect: complete missing --first-parent and --no-checkout options

[Britton Leo Kerin]

The --first-parent and --no-checkout options to the start subcommand of
git-bisect(1) are not completed.

Enable completion of the --first-parent and --no-checkout options to the
start subcommand.  Add test.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash |  2 +-
 t/t9902-completion.sh                  | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6a3d9c7760..57c6e09968 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1478,7 +1478,7 @@ _git_bisect ()
 	start)
 		case "$cur" in
 		--*)
-			__gitcomp "--term-new --term-bad --term-old --term-good"
+			__gitcomp "--first-parent --no-checkout --term-new --term-bad --term-old --term-good"
 			return
 			;;
 		*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 409a5a49d5..f4d3aa67e3 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1271,6 +1271,17 @@ test_expect_success 'git bisect - when not bisecting, complete only replay and s
 	EOF
 '
 
+test_expect_success 'git bisect - complete options to start subcommand' '
+	test_completion "git bisect start --" <<-\EOF
+	--term-new Z
+	--term-bad Z
+	--term-old Z
+	--term-good Z
+	--no-checkout Z
+	--first-parent Z
+	EOF
+'
+
 test_expect_success 'setup for git-bisect tests requiring a repo' '
 	git init git-bisect &&
 	(
-- 
2.43.0

[PATCH v5 5/7] completion: new function __git_complete_log_opts

[Britton Leo Kerin]

The options accepted by git-log are also accepted by at least one other
command (git-bisect).  Factor the common option completion code into
a new function and use it from _git_log.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 57c6e09968..8c3b1b8e96 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2089,10 +2089,12 @@ __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 ()
+# Complete porcelain (i.e. not git-rev-list) options and at least some
+# option arguments accepted by git-log.  Note that this same set of options
+# are also accepted by some other git commands besides git-log.
+__git_complete_log_opts ()
 {
-	__git_has_doubledash && return
-	__git_find_repo_path
+        COMPREPLY=""
 
 	local merge=""
 	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
@@ -2186,6 +2188,16 @@ _git_log ()
 		return
 		;;
 	esac
+}
+
+_git_log ()
+{
+	__git_has_doubledash && return
+	__git_find_repo_path
+
+	__git_complete_log_opts
+	[ -z "$COMPREPLY" ] || return
+
 	__git_complete_revlist
 }
 
-- 
2.43.0

Re: [PATCH v5 5/7] completion: new function __git_complete_log_opts

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]

On Mon, Feb 05, 2024 at 05:09:28PM -0900, Britton Leo Kerin wrote:
> The options accepted by git-log are also accepted by at least one other
> command (git-bisect).  Factor the common option completion code into
> a new function and use it from _git_log.
> 
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 57c6e09968..8c3b1b8e96 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2089,10 +2089,12 @@ __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 ()
> +# Complete porcelain (i.e. not git-rev-list) options and at least some
> +# option arguments accepted by git-log.  Note that this same set of options
> +# are also accepted by some other git commands besides git-log.
> +__git_complete_log_opts ()
>  {
> -	__git_has_doubledash && return
> -	__git_find_repo_path
> +        COMPREPLY=""

Nit: this is indented with spaces instead of tabs.

It would also be nice to mention in the commit message why we empty
COMPREPLY here. It didn't happen before either, so this is essentially
not a no-op conversion.

Patrick

>  
>  	local merge=""
>  	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> @@ -2186,6 +2188,16 @@ _git_log ()
>  		return
>  		;;
>  	esac
> +}
> +
> +_git_log ()
> +{
> +	__git_has_doubledash && return
> +	__git_find_repo_path
> +
> +	__git_complete_log_opts
> +	[ -z "$COMPREPLY" ] || return
> +
>  	__git_complete_revlist
>  }
>  
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 6/7] completion: bisect: complete log opts for visualize subcommand

[Britton Leo Kerin]

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.

Make completion of porcelain git-log options and option arguments to the
visualize subcommand work by calling __git_complete_log_opts when the
start of an option to the subcommand is seen (visualize doesn't support
any options besides the git-log options).  Add test.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash |  4 ++++
 t/t9902-completion.sh                  | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8c3b1b8e96..b4cd94182e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1490,6 +1490,10 @@ _git_bisect ()
 		__gitcomp "--term-good --term-old --term-bad --term-new"
 		return
 		;;
+	visualize)
+		__git_complete_log_opts
+		return
+		;;
 	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
 		__git_complete_refs
 		;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f4d3aa67e3..74132699b1 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1360,6 +1360,20 @@ test_expect_success 'git-bisect - options to terms subcommand are candidates' '
 	)
 '
 
+test_expect_success 'git-bisect - git-log options to visualize subcommand are candidates' '
+	(
+		cd git-bisect &&
+		# The completion used for git-log and here does not complete
+		# every git-log option, so rather than hope to stay in sync
+		# with exactly what it does we will just spot-test here.
+		test_completion "git bisect visualize --sta" <<-\EOF &&
+		--stat Z
+		EOF
+		test_completion "git bisect visualize --summar" <<-\EOF
+		--summary Z
+		EOF
+	)
+'
 
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
-- 
2.43.0

[PATCH v5 7/7] completion: bisect: recognize but do not complete view subcommand

[Britton Leo Kerin]

The "view" alias for the visualize subcommand is neither completed nor
recognized.  It's undesirable to complete it because it's first letters
are the same as for visualize, making completion less rather than more
efficient without adding much in the way of interface discovery.
However, it needs to be recognized in order to enable log option
completion for it.

Recognize but do not complete the view command by creating and using
separate lists of completable_subcommands and all_subcommands.  Add
tests.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 +++++++++++----
 t/t9902-completion.sh                  | 24 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b4cd94182e..b3d5468c15 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1462,12 +1462,19 @@ _git_bisect ()
 	# 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.
-	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
-	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	#
+	# 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 "$all_subcommands")"
+
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
 		if [ -f "$__git_repo_path"/BISECT_START ]; then
-			__gitcomp "$subcommands"
+			__gitcomp "$completable_subcommands"
 		else
 			__gitcomp "replay start"
 		fi
@@ -1490,7 +1497,7 @@ _git_bisect ()
 		__gitcomp "--term-good --term-old --term-bad --term-new"
 		return
 		;;
-	visualize)
+	visualize|view)
 		__git_complete_log_opts
 		return
 		;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 74132699b1..6a6019b0a8 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1375,6 +1375,30 @@ test_expect_success 'git-bisect - git-log options to visualize subcommand are ca
 	)
 '
 
+test_expect_success 'git-bisect - view subcommand is not a candidate' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect vi" <<-\EOF
+		visualize Z
+		EOF
+	)
+'
+
+test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
+	(
+		cd git-bisect &&
+		# The completion used for git-log and here does not complete
+		# every git-log option, so rather than hope to stay in sync
+		# with exactly what it does we will just spot-test here.
+		test_completion "git bisect view --sta" <<-\EOF &&
+		--stat Z
+		EOF
+		test_completion "git bisect view --summar" <<-\EOF
+		--summary Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

Re: [PATCH v5 0/7] completion: improvements for git-bisect

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

On Mon, Feb 05, 2024 at 05:09:23PM -0900, Britton Leo Kerin wrote:
> Relative to v4 this make the following actual changes:
> 
>   * fixes GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME to 'master' for all of
>     t9902-completion.sh as suggested by Junio.  This change affects all
>     of t9902-completion.sh so I've put it by itself in it's own commit.
> 
>   * uses BISECT_TERMS to avoid pointless processes as suggested by Patrick.
> 
> The commits are also refactored as follows:
> 
>   * squashes the introduction of __git_complete_log_opts in with it's
>     first use a suggested by Patrick.
> 
>   * spreads tests across commits as suggest by Patrick.
> 
> Thanks for the reviews.

This version looks great to me, thanks! I have a last set of nits to
bring this over the finish line (at least from my perspective). Each one
of them on its own wouldn't be worth addressing, but combined I think it
does make sense to send out a new version to address them.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v6 0/7] completion: improvements for git-bisect

[Britton Leo Kerin]

Relative to v5 this makes the following actual changes:

  * Treat COMPREPLY as an array on assignment and test, rather than
    relying on the bash mechanism of implicitly acting on the first
    element of the array.

  * Whitespace fixes.

The commit message about __git_complete_log_opts has also been changed
to indicate that COMPREPLY is emptied and why, and a broken
Signed-off-by line fixed.

Britton Leo Kerin (7):
  completion: tests: always use 'master' for default initial branch name
  completion: bisect: complete bad, new, old, and help subcommands
  completion: bisect: complete custom terms and related options
  completion: bisect: complete missing --first-parent and --no-checkout
    options
  completion: new function __git_complete_log_opts
  completion: bisect: complete log opts for visualize subcommand
  completion: bisect: recognize but do not complete view subcommand

 contrib/completion/git-completion.bash |  65 ++++++++++--
 t/t9902-completion.sh                  | 141 +++++++++++++++++++++++++
 2 files changed, 199 insertions(+), 7 deletions(-)

Range-diff against v5:
1:  71b73de914 = 1:  71b73de914 completion: tests: always use 'master' for default initial branch name
2:  3a478a7a08 ! 2:  7bc45bfc13 completion: bisect: complete bad, new, old, and help subcommands
    @@ Commit message
         such that the commands and their possible ref arguments are completed.
         Add tests.

    -    Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.c
    +    Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

      ## contrib/completion/git-completion.bash ##
     @@ contrib/completion/git-completion.bash: _git_bisect ()
3:  fab7159cf4 ! 3:  be925327d3 completion: bisect: complete custom terms and related options
    @@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subc
      		reset Z
      		visualize Z
     @@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
    - 		EOF
      	)
      '
    +
     +test_expect_success 'git-bisect - options to terms subcommand are candidates' '
     +	(
     +		cd git-bisect &&
    @@ t/t9902-completion.sh: test_expect_success 'git-bisect - when bisecting all subc
     +	)
     +'
     +
    -
      test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
      	test_completion "git checkout " <<-\EOF
    + 	HEAD Z
4:  73f3343b94 = 4:  c3141921e5 completion: bisect: complete missing --first-parent and --no-checkout options
5:  a20846bbd3 ! 5:  092bfba6b1 completion: new function __git_complete_log_opts
    @@ Commit message
         completion: new function __git_complete_log_opts

         The options accepted by git-log are also accepted by at least one other
    -    command (git-bisect).  Factor the common option completion code into
    -    a new function and use it from _git_log.
    +    command (git-bisect).  Factor the common option completion code into a
    +    new function and use it from _git_log.  The new function leaves
    +    COMPREPLY empty if no option candidates are found, so that callers can
    +    safely check it to determine if completion for other arguments should be
    +    attempted.

         Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>

    @@ contrib/completion/git-completion.bash: __git_diff_merges_opts="off none on firs
      {
     -	__git_has_doubledash && return
     -	__git_find_repo_path
    -+        COMPREPLY=""
    ++	COMPREPLY=()

      	local merge=""
      	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
    @@ contrib/completion/git-completion.bash: _git_log ()
     +	__git_find_repo_path
     +
     +	__git_complete_log_opts
    -+	[ -z "$COMPREPLY" ] || return
    ++        [ ${#COMPREPLY[@]} -eq 0 ] || return
     +
      	__git_complete_revlist
      }
6:  fe5545c9a3 ! 6:  9afd4d4e0f completion: bisect: complete log opts for visualize subcommand
    @@ t/t9902-completion.sh: test_expect_success 'git-bisect - options to terms subcom
     +		EOF
     +	)
     +'
    -
    ++
      test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
      	test_completion "git checkout " <<-\EOF
    + 	HEAD Z
7:  c9102ac532 = 7:  dba916b31c completion: bisect: recognize but do not complete view subcommand
--
2.43.0

[PATCH v6 1/7] completion: tests: always use 'master' for default initial branch name

[Britton Leo Kerin]

The default initial branch name can normally be configured using the
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME environment variable.  However,
when testing e.g. <rev> completion it's convenient to know the
exact initial branch name that will be used.

To achieve that without too much trouble it is considered sufficient
to force the default initial branch name to 'master' for all of
t9902-completion.sh.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 t/t9902-completion.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..a5d4e900a2 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -5,6 +5,11 @@
 
 test_description='test bash completion'
 
+# Override environment and always use master for the default initial branch
+# name for these tests, so that rev completion candidates are as expected.
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
 . ./lib-bash.sh
 
 complete ()
-- 
2.43.0

[PATCH v6 2/7] completion: bisect: complete bad, new, old, and help subcommands

[Britton Leo Kerin]

The bad, new, old and help subcommands to git-bisect(1) are not
completed.

Add the bad, new, old, and help subcommands to the appropriate lists
such that the commands and their possible ref arguments are completed.
Add tests.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash |  4 +-
 t/t9902-completion.sh                  | 71 ++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..06d0b156e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad good skip reset visualize replay log run"
+	local subcommands="start bad new good old skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1462,7 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|good|reset|skip|start)
+	bad|new|good|old|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a5d4e900a2..7388c892cf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1264,6 +1264,77 @@ test_expect_success 'git switch - with no options, complete local branches and u
 	EOF
 '
 
+test_expect_success 'git bisect - when not bisecting, complete only replay and start subcommands' '
+	test_completion "git bisect " <<-\EOF
+	replay Z
+	start Z
+	EOF
+'
+
+test_expect_success 'setup for git-bisect tests requiring a repo' '
+	git init git-bisect &&
+	(
+		cd git-bisect &&
+		echo "initial contents" >file &&
+		git add file &&
+		git commit -am "Initial commit" &&
+		git tag initial &&
+		echo "new line" >>file &&
+		git commit -am "First change" &&
+		echo "another new line" >>file &&
+		git commit -am "Second change" &&
+		git tag final
+	)
+'
+
+test_expect_success 'git bisect - start subcommand arguments before double-dash are completed as revs' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect start " <<-\EOF
+		HEAD Z
+		final Z
+		initial Z
+		master Z
+		EOF
+	)
+'
+
+# Note that these arguments are <pathspec>s, which in practice the fallback
+# completion (not the git completion) later ends up completing as paths.
+test_expect_success 'git bisect - start subcommand arguments after double-dash are not completed' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect start final initial -- " ""
+	)
+'
+
+test_expect_success 'setup for git-bisect tests requiring ongoing bisection' '
+	(
+		cd git-bisect &&
+		git bisect start --term-new=custom_new --term-old=custom_old final initial
+	)
+'
+
+test_expect_success 'git-bisect - when bisecting all subcommands are candidates' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect " <<-\EOF
+		start Z
+		bad Z
+		new Z
+		good Z
+		old Z
+		skip Z
+		reset Z
+		visualize Z
+		replay Z
+		log Z
+		run Z
+		help Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

[PATCH v6 3/7] completion: bisect: complete custom terms and related options

[Britton Leo Kerin]

git bisect supports the use of custom terms via the --term-(new|bad) and
--term-(old|good) options, but the completion code doesn't know about
these options or the new subcommands they define.

Add support for these options and the custom subcommands by checking for
BISECT_TERMS and adding them to the list of subcommands.  Add tests.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++--
 t/t9902-completion.sh                  | 15 ++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 06d0b156e7..6a3d9c7760 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,20 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad new good old skip reset visualize replay log run help"
+	__git_find_repo_path
+
+	# If a bisection is in progress get the terms being used.
+	local term_bad term_good
+	if [ -f "$__git_repo_path"/BISECT_TERMS ]; then
+		term_bad=$(__git bisect terms --term-bad)
+		term_good=$(__git bisect terms --term-good)
+	fi
+
+	# 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.
+	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1475,22 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|new|good|old|reset|skip|start)
+	start)
+		case "$cur" in
+		--*)
+			__gitcomp "--term-new --term-bad --term-old --term-good"
+			return
+			;;
+		*)
+			__git_complete_refs
+			;;
+		esac
+		;;
+	terms)
+		__gitcomp "--term-good --term-old --term-bad --term-new"
+		return
+		;;
+	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
 		__git_complete_refs
 		;;
 	*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7388c892cf..304903b1a7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1321,9 +1321,12 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
 		test_completion "git bisect " <<-\EOF
 		start Z
 		bad Z
+		custom_new Z
+		custom_old Z
 		new Z
 		good Z
 		old Z
+		terms Z
 		skip Z
 		reset Z
 		visualize Z
@@ -1335,6 +1338,18 @@ test_expect_success 'git-bisect - when bisecting all subcommands are candidates'
 	)
 '
 
+test_expect_success 'git-bisect - options to terms subcommand are candidates' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect terms --" <<-\EOF
+		--term-bad Z
+		--term-good Z
+		--term-new Z
+		--term-old Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

[PATCH v6 4/7] completion: bisect: complete missing --first-parent and --no-checkout options

[Britton Leo Kerin]

The --first-parent and --no-checkout options to the start subcommand of
git-bisect(1) are not completed.

Enable completion of the --first-parent and --no-checkout options to the
start subcommand.  Add test.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash |  2 +-
 t/t9902-completion.sh                  | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6a3d9c7760..57c6e09968 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1478,7 +1478,7 @@ _git_bisect ()
 	start)
 		case "$cur" in
 		--*)
-			__gitcomp "--term-new --term-bad --term-old --term-good"
+			__gitcomp "--first-parent --no-checkout --term-new --term-bad --term-old --term-good"
 			return
 			;;
 		*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 304903b1a7..8fcd1cfa7e 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1271,6 +1271,17 @@ test_expect_success 'git bisect - when not bisecting, complete only replay and s
 	EOF
 '
 
+test_expect_success 'git bisect - complete options to start subcommand' '
+	test_completion "git bisect start --" <<-\EOF
+	--term-new Z
+	--term-bad Z
+	--term-old Z
+	--term-good Z
+	--no-checkout Z
+	--first-parent Z
+	EOF
+'
+
 test_expect_success 'setup for git-bisect tests requiring a repo' '
 	git init git-bisect &&
 	(
-- 
2.43.0

[PATCH v6 5/7] completion: new function __git_complete_log_opts

[Britton Leo Kerin]

The options accepted by git-log are also accepted by at least one other
command (git-bisect).  Factor the common option completion code into a
new function and use it from _git_log.  The new function leaves
COMPREPLY empty if no option candidates are found, so that callers can
safely check it to determine if completion for other arguments should be
attempted.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 57c6e09968..b9ebd5e409 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2089,10 +2089,12 @@ __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 ()
+# Complete porcelain (i.e. not git-rev-list) options and at least some
+# option arguments accepted by git-log.  Note that this same set of options
+# are also accepted by some other git commands besides git-log.
+__git_complete_log_opts ()
 {
-	__git_has_doubledash && return
-	__git_find_repo_path
+	COMPREPLY=()
 
 	local merge=""
 	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
@@ -2186,6 +2188,16 @@ _git_log ()
 		return
 		;;
 	esac
+}
+
+_git_log ()
+{
+	__git_has_doubledash && return
+	__git_find_repo_path
+
+	__git_complete_log_opts
+        [ ${#COMPREPLY[@]} -eq 0 ] || return
+
 	__git_complete_revlist
 }
 
-- 
2.43.0

[PATCH v6 6/7] completion: bisect: complete log opts for visualize subcommand

[Britton Leo Kerin]

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.

Make completion of porcelain git-log options and option arguments to the
visualize subcommand work by calling __git_complete_log_opts when the
start of an option to the subcommand is seen (visualize doesn't support
any options besides the git-log options).  Add test.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash |  4 ++++
 t/t9902-completion.sh                  | 15 +++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b9ebd5e409..5337ae4ce2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1490,6 +1490,10 @@ _git_bisect ()
 		__gitcomp "--term-good --term-old --term-bad --term-new"
 		return
 		;;
+	visualize)
+		__git_complete_log_opts
+		return
+		;;
 	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip)
 		__git_complete_refs
 		;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 8fcd1cfa7e..b989388e7e 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1361,6 +1361,21 @@ test_expect_success 'git-bisect - options to terms subcommand are candidates' '
 	)
 '
 
+test_expect_success 'git-bisect - git-log options to visualize subcommand are candidates' '
+	(
+		cd git-bisect &&
+		# The completion used for git-log and here does not complete
+		# every git-log option, so rather than hope to stay in sync
+		# with exactly what it does we will just spot-test here.
+		test_completion "git bisect visualize --sta" <<-\EOF &&
+		--stat Z
+		EOF
+		test_completion "git bisect visualize --summar" <<-\EOF
+		--summary Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

[PATCH v6 7/7] completion: bisect: recognize but do not complete view subcommand

[Britton Leo Kerin]

The "view" alias for the visualize subcommand is neither completed nor
recognized.  It's undesirable to complete it because it's first letters
are the same as for visualize, making completion less rather than more
efficient without adding much in the way of interface discovery.
However, it needs to be recognized in order to enable log option
completion for it.

Recognize but do not complete the view command by creating and using
separate lists of completable_subcommands and all_subcommands.  Add
tests.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 +++++++++++----
 t/t9902-completion.sh                  | 24 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5337ae4ce2..0734debc11 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1462,12 +1462,19 @@ _git_bisect ()
 	# 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.
-	local subcommands="start bad new $term_bad good old $term_good terms skip reset visualize replay log run help"
-	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	#
+	# 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 "$all_subcommands")"
+
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
 		if [ -f "$__git_repo_path"/BISECT_START ]; then
-			__gitcomp "$subcommands"
+			__gitcomp "$completable_subcommands"
 		else
 			__gitcomp "replay start"
 		fi
@@ -1490,7 +1497,7 @@ _git_bisect ()
 		__gitcomp "--term-good --term-old --term-bad --term-new"
 		return
 		;;
-	visualize)
+	visualize|view)
 		__git_complete_log_opts
 		return
 		;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index b989388e7e..70557eb684 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1376,6 +1376,30 @@ test_expect_success 'git-bisect - git-log options to visualize subcommand are ca
 	)
 '
 
+test_expect_success 'git-bisect - view subcommand is not a candidate' '
+	(
+		cd git-bisect &&
+		test_completion "git bisect vi" <<-\EOF
+		visualize Z
+		EOF
+	)
+'
+
+test_expect_success 'git-bisect - existing view subcommand is recognized and enables completion of git-log options' '
+	(
+		cd git-bisect &&
+		# The completion used for git-log and here does not complete
+		# every git-log option, so rather than hope to stay in sync
+		# with exactly what it does we will just spot-test here.
+		test_completion "git bisect view --sta" <<-\EOF &&
+		--stat Z
+		EOF
+		test_completion "git bisect view --summar" <<-\EOF
+		--summary Z
+		EOF
+	)
+'
+
 test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
 	test_completion "git checkout " <<-\EOF
 	HEAD Z
-- 
2.43.0

Re: [PATCH v6 0/7] completion: improvements for git-bisect

[Junio C Hamano]

Britton Leo Kerin <britton.kerin@gmail.com> writes:

> Relative to v5 this makes the following actual changes:
>
>   * Treat COMPREPLY as an array on assignment and test, rather than
>     relying on the bash mechanism of implicitly acting on the first
>     element of the array.
>
>   * Whitespace fixes.
>
> The commit message about __git_complete_log_opts has also been changed
> to indicate that COMPREPLY is emptied and why, and a broken
> Signed-off-by line fixed.

Thanks.  Let's queue this version and mark it for 'next' by -rc0.

Re: [PATCH v6 0/7] completion: improvements for git-bisect

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

On Tue, Feb 06, 2024 at 02:58:06PM -0800, Junio C Hamano wrote:
> Britton Leo Kerin <britton.kerin@gmail.com> writes:
> 
> > Relative to v5 this makes the following actual changes:
> >
> >   * Treat COMPREPLY as an array on assignment and test, rather than
> >     relying on the bash mechanism of implicitly acting on the first
> >     element of the array.
> >
> >   * Whitespace fixes.
> >
> > The commit message about __git_complete_log_opts has also been changed
> > to indicate that COMPREPLY is emptied and why, and a broken
> > Signed-off-by line fixed.
> 
> Thanks.  Let's queue this version and mark it for 'next' by -rc0.

The range-diff looks good to me indeed, so I agree that this can be
queued. Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 1/5] completion: complete new old actions, start opts

[Britton Leo Kerin]

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..15d22ff7d9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1449,7 +1449,7 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad good skip reset visualize replay log run"
+	local subcommands="start bad new good old terms skip reset visualize replay log run help"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__git_find_repo_path
@@ -1462,7 +1462,20 @@ _git_bisect ()
 	fi
 
 	case "$subcommand" in
-	bad|good|reset|skip|start)
+	start)
+		case "$cur" in
+		--*)
+			__gitcomp "--term-new --term-bad --term-old --term-good --first-parent --no-checkout"
+			return
+			;;
+		*)
+			;;
+		esac
+		;;
+	esac
+
+	case "$subcommand" in
+	bad|new|good|old|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
-- 
2.43.0

[PATCH v2 2/5] completion: git-log opts to bisect visualize

[Britton Leo Kerin]

To do this the majority of _git_log has been factored out into the new
__git_complete_log_opts.  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@gmail.com>
---
 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
+		;;
 	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.
+__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
+
+        __git_complete_log_opts
+        [ -z "$COMPREPLY" ] || return
+
 	__git_complete_revlist
 }
 
-- 
2.43.0

[PATCH v2 3/5] completion: move to maintain define-before-use

[Britton Leo Kerin]

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 269 ++++++++++++-------------
 1 file changed, 134 insertions(+), 135 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c16aded36c..63ca8082a4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1445,6 +1445,140 @@ _git_archive ()
 	__git_complete_file
 }
 
+# Options that go well for log, shortlog and gitk
+__git_log_common_options="
+	--not --all
+	--branches --tags --remotes
+	--first-parent --merges --no-merges
+	--max-count=
+	--max-age= --since= --after=
+	--min-age= --until= --before=
+	--min-parents= --max-parents=
+	--no-min-parents --no-max-parents
+"
+# Options that go well for log and gitk (not shortlog)
+__git_log_gitk_options="
+	--dense --sparse --full-history
+	--simplify-merges --simplify-by-decoration
+	--left-right --notes --no-notes
+"
+# Options that go well for log and shortlog (not gitk)
+__git_log_shortlog_options="
+	--author= --committer= --grep=
+	--all-match --invert-grep
+"
+# Options accepted by log and show
+__git_log_show_options="
+	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
+"
+
+__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
+
+__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:"
+
+# 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.
+__git_complete_log_opts ()
+{
+	[ -z "$COMPREPLY" ] || return 1   # Precondition
+
+	local merge=""
+	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
+		merge="--merge"
+	fi
+	case "$prev,$cur" in
+	-L,:*:*)
+		return	# fall back to Bash filename completion
+		;;
+	-L,:*)
+		__git_complete_symbol --cur="${cur#:}" --sfx=":"
+		return
+		;;
+	-G,*|-S,*)
+		__git_complete_symbol
+		return
+		;;
+	esac
+	case "$cur" in
+	--pretty=*|--format=*)
+		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
+			" "" "${cur#*=}"
+		return
+		;;
+	--date=*)
+		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
+		return
+		;;
+	--decorate=*)
+		__gitcomp "full short no" "" "${cur##--decorate=}"
+		return
+		;;
+	--diff-algorithm=*)
+		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
+		return
+		;;
+	--submodule=*)
+		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
+		return
+		;;
+	--ws-error-highlight=*)
+		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
+		return
+		;;
+	--no-walk=*)
+		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
+		return
+		;;
+	--diff-merges=*)
+                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
+                return
+                ;;
+	--*)
+		__gitcomp "
+			$__git_log_common_options
+			$__git_log_shortlog_options
+			$__git_log_gitk_options
+			$__git_log_show_options
+			--root --topo-order --date-order --reverse
+			--follow --full-diff
+			--abbrev-commit --no-abbrev-commit --abbrev=
+			--relative-date --date=
+			--pretty= --format= --oneline
+			--show-signature
+			--cherry-mark
+			--cherry-pick
+			--graph
+			--decorate --decorate= --no-decorate
+			--walk-reflogs
+			--no-walk --no-walk= --do-walk
+			--parents --children
+			--expand-tabs --expand-tabs= --no-expand-tabs
+			$merge
+			$__git_diff_common_options
+			"
+		return
+		;;
+	-L:*:*)
+		return	# fall back to Bash filename completion
+		;;
+	-L:*)
+		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
+		return
+		;;
+	-G*)
+		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
+		return
+		;;
+	-S*)
+		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
+		return
+		;;
+	esac
+}
+
 _git_bisect ()
 {
 	__git_has_doubledash && return
@@ -2052,141 +2186,6 @@ _git_ls_tree ()
 	__git_complete_file
 }
 
-# Options that go well for log, shortlog and gitk
-__git_log_common_options="
-	--not --all
-	--branches --tags --remotes
-	--first-parent --merges --no-merges
-	--max-count=
-	--max-age= --since= --after=
-	--min-age= --until= --before=
-	--min-parents= --max-parents=
-	--no-min-parents --no-max-parents
-"
-# Options that go well for log and gitk (not shortlog)
-__git_log_gitk_options="
-	--dense --sparse --full-history
-	--simplify-merges --simplify-by-decoration
-	--left-right --notes --no-notes
-"
-# Options that go well for log and shortlog (not gitk)
-__git_log_shortlog_options="
-	--author= --committer= --grep=
-	--all-match --invert-grep
-"
-# Options accepted by log and show
-__git_log_show_options="
-	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
-"
-
-__git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
-
-__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:"
-
-
-# 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.
-__git_complete_log_opts ()
-{
-	[ -z "$COMPREPLY" ] || return 1   # Precondition
-
-	local merge=""
-	if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
-		merge="--merge"
-	fi
-	case "$prev,$cur" in
-	-L,:*:*)
-		return	# fall back to Bash filename completion
-		;;
-	-L,:*)
-		__git_complete_symbol --cur="${cur#:}" --sfx=":"
-		return
-		;;
-	-G,*|-S,*)
-		__git_complete_symbol
-		return
-		;;
-	esac
-	case "$cur" in
-	--pretty=*|--format=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur#*=}"
-		return
-		;;
-	--date=*)
-		__gitcomp "$__git_log_date_formats" "" "${cur##--date=}"
-		return
-		;;
-	--decorate=*)
-		__gitcomp "full short no" "" "${cur##--decorate=}"
-		return
-		;;
-	--diff-algorithm=*)
-		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
-		return
-		;;
-	--submodule=*)
-		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
-		return
-		;;
-	--ws-error-highlight=*)
-		__gitcomp "$__git_ws_error_highlight_opts" "" "${cur##--ws-error-highlight=}"
-		return
-		;;
-	--no-walk=*)
-		__gitcomp "sorted unsorted" "" "${cur##--no-walk=}"
-		return
-		;;
-	--diff-merges=*)
-                __gitcomp "$__git_diff_merges_opts" "" "${cur##--diff-merges=}"
-                return
-                ;;
-	--*)
-		__gitcomp "
-			$__git_log_common_options
-			$__git_log_shortlog_options
-			$__git_log_gitk_options
-			$__git_log_show_options
-			--root --topo-order --date-order --reverse
-			--follow --full-diff
-			--abbrev-commit --no-abbrev-commit --abbrev=
-			--relative-date --date=
-			--pretty= --format= --oneline
-			--show-signature
-			--cherry-mark
-			--cherry-pick
-			--graph
-			--decorate --decorate= --no-decorate
-			--walk-reflogs
-			--no-walk --no-walk= --do-walk
-			--parents --children
-			--expand-tabs --expand-tabs= --no-expand-tabs
-			$merge
-			$__git_diff_common_options
-			"
-		return
-		;;
-	-L:*:*)
-		return	# fall back to Bash filename completion
-		;;
-	-L:*)
-		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
-		return
-		;;
-	-G*)
-		__git_complete_symbol --pfx="-G" --cur="${cur#-G}"
-		return
-		;;
-	-S*)
-		__git_complete_symbol --pfx="-S" --cur="${cur#-S}"
-		return
-		;;
-	esac
-}
-
 _git_log ()
 {
 	__git_has_doubledash && return
-- 
2.43.0

[PATCH v2 4/5] completion: custom git-bisect terms

[Britton Leo Kerin]

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 63ca8082a4..ad80df6630 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1583,10 +1583,19 @@ _git_bisect ()
 {
 	__git_has_doubledash && return
 
-	local subcommands="start bad new good old terms skip reset visualize replay log run help"
+	__git_find_repo_path
+
+	local term_bad term_good
+	if [ -f "$__git_repo_path"/BISECT_START ]; then
+		term_bad=`__git bisect terms --term-bad`
+		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"
+
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
 	if [ -z "$subcommand" ]; then
-		__git_find_repo_path
 		if [ -f "$__git_repo_path"/BISECT_START ]; then
 			__gitcomp "$subcommands"
 		else
@@ -1619,7 +1628,7 @@ _git_bisect ()
 	esac
 
 	case "$subcommand" in
-	bad|new|good|old|reset|skip|start)
+	bad|new|"$term_bad"|good|old|"$term_good"|reset|skip|start)
 		__git_complete_refs
 		;;
 	*)
-- 
2.43.0

[PATCH v2 5/5] completion: custom git-bisect terms

[Britton Leo Kerin]

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.
+	#
+	# 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