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

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

Do you foreseen such new parameters?

> I don't really have a good idea for how to fix that reliably though, I
> have to admit. Maybe we should just mostly ignore this edge case and
> always complete references, unless we have seen a `--`? That can be
> checked rather easily via `__git_hash_doubledash`.

My toughs are that if such a special case ever surface, we can just
dispatch it earlier before we check for the
__git_history_has_revision, like this:

---8<---
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d313780d8b..786fcb5e16 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2193,6 +2193,15 @@ _git_history ()
 		esac
 	fi
 
+	# Subcommands which takes something else than a revision
+	case "$subcommand" in
+	foo)
+		# 'git history foo' take a file first
+		__git_complete_index_file "--cached"
+		return
+		;;
+	esac
+
 	if ! __git_history_has_revision; then
 		__git_complete_refs
 		return
---8<---

This seems reasonable to me. Once we know what this mysterious new
command would be, maybe we can find a smarter and more tailored
solution, but at the moment, I would not call this a blocker.

> That'd still be a huge win compared to the status quo, and if we really
> care about making this work properly we can still iterate.

Thanks!


Yours sincerely,
Vincent Mailhol
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.