Re: [PATCH] gitk: fix --all behavior combined with --not

[Junio C Hamano <gitster@pobox.com>]

Heiko Voigt <hvoigt@hvoigt.net> writes:

> behavior. How about '--all-include-head'. Then e.g.
>
>     git rev-parse --all-include-head --all --not origin/master
>
> would include the head ref like you proposed below?
>
> What do you think? Or would you rather go the route of changing
> rev-parse behavior?

Depends on what you mean by the above.  Do you mean that now the end
user needs to say

	gitk --all-include-head --not origin/master

to get a rough equivalent of

	git log --graph --oneline --all --not origin/master

due to the discrepancy between how "rev-parse" and "rev-list" treat
their "--all" option?  Or do you mean that the end user still says
"--all", and after (reliably by some means) making sure that "--all"
given by the end-user is a request for "all refs and HEAD", we turn
that into the above internal rev-parse call?

If the former, then quite honestly, we shouldn't doing anything,
perhaps other than reverting 4d5e1b1319.  The users can type

	$ gitk --all HEAD --not origin/master
	$ gitk $commit --not --all HEAD

themselves, instead of --all-include-head.

If the latter, I am not sure what the endgame should be.  

It certainly *is* safer not to unconditionallyl and unilaterally
change the behaviour of "rev-parse --all", so I am all for starting
with small and fully backward compatible change, but wouldn't
scripts other than gitk want the same behaviour?  

To put it the other way around, what use case would we have that we
want to enumerate all refs but not HEAD, *and* exclude HEAD only
when HEAD is detached?  I can see the use of "what are commits
reachable from the current HEAD but not reachable from any of the
refs/*?" and that would be useful whether HEAD is detached or is on
a concrete branch, so "rev-parse --all" that does not include
detached HEAD alone does not feel so useful at least to me.

I am reasonably sure that back when "rev-parse --all" was invented,
the use of detached HEAD was not all that prevalent (I would not be
surprised if it hadn't been invented yet), so it being documented to
enumerate all refs does not necessarily contradict to include HEAD
if it is different from any of the ref tips (i.e. detached).

And if we cannot commit to changing the "rev-parse --all" (and I am
not sure I can at this point---I am wary of changes), as we know
where "--all" appeared on the command line, inserting HEAD immediately
after it at the script level is probably the change with the least
potential damage we can make, without changing anything else.



>
> Cheers Heiko
>
>> 
>>  builtin/rev-parse.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
>> index f8bbe6d47e..94f9a6efba 100644
>> --- a/builtin/rev-parse.c
>> +++ b/builtin/rev-parse.c
>> @@ -766,6 +766,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>>  			}
>>  			if (!strcmp(arg, "--all")) {
>>  				for_each_ref(show_reference, NULL);
>> +				head_ref(show_reference, NULL);
>>  				clear_ref_exclusion(&ref_excludes);
>>  				continue;
>>  			}