threads / rfc / 60677

RFC patch, 6 partscompletion: improvements for git-bisect

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

## tl;dr

58 messages between Jan 2, 2024 and Feb 7, 2024. Diffs are folded; open one to read it.

replies: 57people: 3as markdown or json

Britton Leo Kerin· Jan 2, 2024, 19:57 UTC · lore
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

Britton Leo Kerin· Jan 10, 2024, 02:03 UTC · re: Britton Leo Kerin · lore

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

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

Britton Leo Kerin· Jan 18, 2024, 20:43 UTC · re: Britton Leo Kerin · lore

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

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

Britton Leo Kerin· Jan 18, 2024, 20:43 UTC · re: Britton Leo Kerin · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +15 −2
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
Patrick Steinhardt· Jan 19, 2024, 07:04 UTC · re: Britton Leo Kerin · lore

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

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.

Show 15 quoted lines
> 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".

Show 16 quoted lines
>  	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
Show 12 quoted lines
> +		esac
> +		;;
> +	esac
> +
> +	case "$subcommand" in
> +	bad|new|good|old|reset|skip|start)
>  		__git_complete_refs
>  		;;
>  	*)
> -- 
> 2.43.0
> 
Britton Leo Kerin· Jan 18, 2024, 20:43 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +27 −3
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
Patrick Steinhardt· Jan 19, 2024, 07:04 UTC · re: Britton Leo Kerin · lore

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

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.
Show 33 quoted lines
> 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?

Show 13 quoted lines
>  	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.

Show 18 quoted lines
> +__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
Show 9 quoted lines
> +        __git_complete_log_opts
> +        [ -z "$COMPREPLY" ] || return
> +
>  	__git_complete_revlist
>  }
>  
> -- 
> 2.43.0
> 
Britton Leo Kerin· Jan 18, 2024, 20:43 UTC · re: Britton Leo Kerin · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 269 ++++++++++++-------------
 1 file changed, 134 insertions(+), 135 deletions(-)
Show changes to contrib/completion/git-completion.bash +134 −135
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
Patrick Steinhardt· Jan 19, 2024, 07:04 UTC · re: Britton Leo Kerin · lore

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

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
Show 295 quoted lines
> 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
> 
Britton Leo Kerin· Jan 18, 2024, 20:43 UTC · re: Britton Leo Kerin · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
Show changes to contrib/completion/git-completion.bash +12 −3
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
Patrick Steinhardt· Jan 19, 2024, 07:04 UTC · re: Britton Leo Kerin · lore

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

On Thu, Jan 18, 2024 at 11:43:22AM -0900, Britton Leo Kerin wrote:
Missing an explanation.
Show 21 quoted lines
> 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
Show 21 quoted lines
> +	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
> 
Britton Leo Kerin· Jan 18, 2024, 20:43 UTC · re: Britton Leo Kerin · lore

[PATCH v3 5/5] 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>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Show changes to contrib/completion/git-completion.bash +13 −4
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
Patrick Steinhardt· Jan 19, 2024, 07:05 UTC · re: Britton Leo Kerin · lore

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

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

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

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

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

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

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

On Thu, Jan 18, 2024 at 11:43:18AM -0900, Britton Leo Kerin wrote:
Show 5 quoted lines
> 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
Junio C Hamano· Jan 19, 2024, 18:27 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 17 quoted lines
> 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.
Britton Kerin· Jan 26, 2024, 00:23 UTC · re: Patrick Steinhardt · lore

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

On Thu, Jan 18, 2024 at 10:05 PM Patrick Steinhardt <ps@pks.im> wrote:
Show 14 quoted lines
>
> 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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +30 −2
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
Patrick Steinhardt· Feb 1, 2024, 09:55 UTC · re: Britton Leo Kerin · lore

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

On Sun, Jan 28, 2024 at 01:34:41PM -0900, Britton Leo Kerin wrote:
Show 30 quoted lines
> 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
Show 35 quoted lines
> +	# 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
> 
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

[PATCH v4 4/8] 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 | 101 +++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
Show changes to contrib/completion/git-completion.bash +101 −0
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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +1 −1
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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

[PATCH v4 1/8] 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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +2 −2
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
Patrick Steinhardt· Feb 1, 2024, 09:55 UTC · re: Britton Leo Kerin · lore

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

On Sun, Jan 28, 2024 at 01:34:40PM -0900, Britton Leo Kerin wrote:
Show 33 quoted lines
> 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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +3 −92
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
Patrick Steinhardt· Feb 1, 2024, 09:55 UTC · re: Britton Leo Kerin · lore

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

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
Show 114 quoted lines
> 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
> 
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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(+)
Show changes to contrib/completion/git-completion.bash +4 −0
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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +11 −4
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
Britton Leo Kerin· Jan 28, 2024, 22:34 UTC · re: Britton Leo Kerin · lore

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

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(+)
Show changes to t/t9902-completion.sh +135 −0
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
Junio C Hamano· Jan 30, 2024, 05:47 UTC · re: Britton Leo Kerin · lore

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

Britton Leo Kerin <britton.kerin@gmail.com> writes:
Show 11 quoted lines
> +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):
Show changes to t/t9902-completion.sh +2 −0
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 ()
Patrick Steinhardt· Feb 1, 2024, 09:55 UTC · re: Britton Leo Kerin · lore

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

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
Show 153 quoted lines
> ---
>  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
> 
Patrick Steinhardt· Feb 1, 2024, 09:55 UTC · re: Britton Leo Kerin · lore

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

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
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

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

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
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

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

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(+)
Show changes to t/t9902-completion.sh +5 −0
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
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

[PATCH v5 2/7] 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. 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(-)
Show changes to 2 files +73 −2

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Patrick Steinhardt· Feb 6, 2024, 07:40 UTC · re: Britton Leo Kerin · lore

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

On Mon, Feb 05, 2024 at 05:09:25PM -0900, Britton Leo Kerin wrote:
Show 8 quoted lines
> 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
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to 2 files +45 −2

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Patrick Steinhardt· Feb 6, 2024, 07:40 UTC · re: Britton Leo Kerin · lore

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

On Mon, Feb 05, 2024 at 05:09:26PM -0900, Britton Leo Kerin wrote:
Show 84 quoted lines
> 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...
Show 13 quoted lines
> +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
Show 5 quoted lines
>  test_expect_success 'git checkout - completes refs and unique remote branches for DWIM' '
>  	test_completion "git checkout " <<-\EOF
> -- 
> 2.43.0
> 
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to 2 files +12 −1

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

[PATCH v5 5/7] 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.

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
Show changes to contrib/completion/git-completion.bash +15 −3
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
Patrick Steinhardt· Feb 6, 2024, 07:40 UTC · re: Britton Leo Kerin · lore

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

On Mon, Feb 05, 2024 at 05:09:28PM -0900, Britton Leo Kerin wrote:
Show 26 quoted lines
> 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
Show 23 quoted lines
>  
>  	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
> 
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to 2 files +35 −4

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Britton Leo Kerin· Feb 6, 2024, 02:09 UTC · re: Britton Leo Kerin · lore

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

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(+)
Show changes to 2 files +18 −0

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Patrick Steinhardt· Feb 6, 2024, 07:40 UTC · re: Britton Leo Kerin · lore

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

On Mon, Feb 05, 2024 at 05:09:23PM -0900, Britton Leo Kerin wrote:
Show 16 quoted lines
> 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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

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

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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to 2 files +45 −2

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

[PATCH v6 2/7] 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. 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(-)
Show changes to 2 files +73 −2

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to 2 files +12 −1

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

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

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(+)
Show changes to t/t9902-completion.sh +5 −0
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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

[PATCH v6 5/7] 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. 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(-)
Show changes to contrib/completion/git-completion.bash +15 −3
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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

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

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(-)
Show changes to 2 files +35 −4

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Britton Leo Kerin· Feb 6, 2024, 21:50 UTC · re: Britton Leo Kerin · lore

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

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(+)
Show changes to 2 files +19 −0

contrib/completion/git-completion.bash, t/t9902-completion.sh

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
Junio C Hamano· Feb 6, 2024, 22:58 UTC · re: Britton Leo Kerin · lore

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

Britton Leo Kerin <britton.kerin@gmail.com> writes:
Show 11 quoted lines
> 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.
Patrick Steinhardt· Feb 7, 2024, 05:26 UTC · re: Junio C Hamano · lore

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

On Tue, Feb 06, 2024 at 02:58:06PM -0800, Junio C Hamano wrote:
Show 15 quoted lines
> 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
Britton Leo Kerin· Jan 10, 2024, 02:03 UTC · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
Show changes to contrib/completion/git-completion.bash +12 −3
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
Britton Leo Kerin· Jan 10, 2024, 02:03 UTC · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +15 −2
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
Britton Leo Kerin· Jan 10, 2024, 02:03 UTC · lore

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

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(-)
Show changes to contrib/completion/git-completion.bash +27 −3
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
Britton Leo Kerin· Jan 10, 2024, 02:03 UTC · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 269 ++++++++++++-------------
 1 file changed, 134 insertions(+), 135 deletions(-)
Show changes to contrib/completion/git-completion.bash +134 −135
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
Britton Leo Kerin· Jan 10, 2024, 02:03 UTC · lore

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

Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Show changes to contrib/completion/git-completion.bash +13 −4
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

← back to recent threads