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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2019, 19:01 UTC
Message-ID: <xmqq4l3wz6y8.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190704080907.GA45656@book.hvoigt.net>

Heiko Voigt <hvoigt@hvoigt.net> writes:

> In commit 4d5e1b1319 ("gitk: Show detached HEAD if --all is specified",
> 2014-09-09) the intention was to have detached HEAD shown when the --all
> argument is given.

The "do we have --all?" test added by that old commit is not quite satisfying in the first place. E.g. we do not check if there is a double-dash before it. This change also relies on an ancient design mistake of allowing non-dashed options before a dashed one, adding more to dissatisfaction by making a future change to correct the design mistake harder.

I think in the longer term we should consider changing "git rev-parse --all" to include HEAD in the concept of "all refs" instead. But in the meantime, this patch is not making things drastically wrong, so let's take it as is.

Thanks.

Show 19 quoted lines

>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
>  gitk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk b/gitk
> index a14d7a1..19d95cd 100755
> --- a/gitk
> +++ b/gitk
> @@ -295,7 +295,7 @@ proc parseviewrevs {view revs} {
>      if {$revs eq {}} {
>  	set revs HEAD
>      } elseif {[lsearch -exact $revs --all] >= 0} {
> -	lappend revs HEAD
> +	linsert revs 0 HEAD
>      }
>      if {[catch {set ids [eval exec git rev-parse $revs]} err]} {
>  	# we get stdout followed by stderr in $err