Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation

From: Johannes Sixt <j6t@kdbg.org>
Date: May 23, 2026, 20:31 UTC
Message-ID: <8b4e8b73-eb2c-41bb-9653-08e38fedd434@kdbg.org>
In-Reply-To: <20260520202411.108764-11-mlevedahl@gmail.com>

Am 20.05.26 um 22:24 schrieb Mark Levedahl:

Show 24 quoted lines

> git-gui's blame and browser subcommands do not work with bare
> repositories, but they should per commit c52c94524b ("git-gui: Allow
> blame/browser subcommands on bare repositories", 2007-07-17). Assuming
> that commit worked, something changed since reintroducing a hard-coded
> dependency upon a worktree.
> 
> The basic issue goes back to 3e45ee1ef2 ("git-gui: Smarter command line
> parsing for browser, blame", 2007-05-08), which seeks to implement
> command line parsing similar to git blame. That commit introduces
> depencies upon the worktree to decide which argument is rev or path.
> 
> Looking at builtin/blame.c in git around line 1120:
> 
> 	 * (1) if dashdash_pos != 0, it is either
> 	 *     "blame [revisions] -- <path>" or
> 	 *     "blame -- <path> <rev>"
> 	 *
> 	 * (2) otherwise, it is one of the two:
> 	 *     "blame [revisions] <path>"
> 	 *     "blame <path> <rev>"
> 
> shows the clear intent: rev and path may be swapped in input so both
> meanings must be tried, but -- may be used to designate which is the
> path forcing or precluding trying the swapped arguments.

Please do not use this code comment as recipe for our own argument parseing. In particular, that <path> can occur for <rev> goes back to the initial implementation of git pickaxe in cee7f245dcae ("git-pickaxe: blame rewritten.", 2006-10-19). Since acca687fa9db ("git-pickaxe: retire pickaxe", 2006-11-08), the documentation of git-blame states that <file> is always last (but the implementation was not adjusted accordingly).

In general, Git's argument parsing requires revisions before pathspec. To disambiguate, '--' can be used. If it is not used, arguments are check whether they are files or refs, and as soon as one argument is identified as file unambiguously, all later arguments must also be files.

We should follow this pattern, and to do that, we could just delegate argument processing to `git rev-parse`.

Show 8 quoted lines

> 
> With a worktree, git gui correctly swaps the arguments if the given path
> exists in the worktree. git blame does this using the git repository.
> But, git-gui sometimes interprets the -- to have an exactly opposite
> meaning:
> 
>     git blame       Makefile gitgui-0.19.0       works
>     git gui blame   Makefile gitgui-0.19.0       works

Git gui shows something, but ignores the ref, so doesn't quite work.

> 
>     git blame       -- Makefile gitgui-0.19.0    works
>     git gui blame   -- Makefile gitgui-0.19.0    works

Ditto.

> 
>     git blame       Makefile -- gitgui-0.19.0    fails (correctly)
>     git gui blame   Makefile -- gitgui-0.19.0    works (should fail)

Ditto.

> 
>     git blame       gitgui-0.19.0 -- Makefile    works (correctly)
>     git gui blame   gitgui-0.19.0 -- Makefile    fails (should work)

Yes, there's a bug in the argument parser that -- isn't skipped, but treated as the file name.

Show 6 quoted lines

> 
> It is possible to patch the code to operate without a worktree, but this
> will make the commands operate differently with and without a worktree,
> won't fix the parsing issues above, and won't address the issues that
> can arise when using a worktree to help decisions on a different rev
> with file/directory conflicts, etc.

Before this patch 'git gui blame' can show contents uncommitted changes, but with this patch this isn't possible. I see you have just sent a patch that may fix this, but I havn't looked at it, yet.

Show 7 quoted lines

> 
> So, let's rework the parser so that it uses -- as does git blame, and
> uses git ls-tree to query the given revision for existence and type of
> path rather than basing this upon a possibly unrelated worktree. Also,
> abort early when the given path is not found, or does not match the need
> (file or directory). This fixes some current cases where git-gui will
> open a window with no content, possibly also with an error message.

There is no desire to make 'git gui blame' work the same with and without a working tree.

If we invoke git to help argument parsing, then it should be 'rev-parse', not 'ls-tree'.

Show 6 quoted lines

> 
> This does not change whether or how git-gui uses staged and unstaged
> content in the current worktree for blame display.
> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---

>  	wm deiconify .
>  	switch -- $subcommand {
>  	browser {
> -		if {$jump_spec ne {}} usage

Let's keep this line, which diagnoses an incorrect --line= argument.

Show 24 quoted lines

> -		if {$head eq {}} {
> -			if {$path ne {} && [file isdirectory $path]} {
> -				set head $current_branch
> -			} else {
> -				set head $path
> -				set path {}
> -			}
> -		}
>  		browser::new $head $path
>  	}
> -	blame   {
> -		if {$head eq {} && ![file exists $path]} {
> -			catch {wm withdraw .}
> -			tk_messageBox \
> -				-icon error \
> -				-type ok \
> -				-title [mc "git-gui: fatal error"] \
> -				-message [mc "fatal: cannot stat path %s: No such file or directory" $path]
> -			exit 1
> -		}
> +	blame {
>  		blame::new $head $path $jump_spec
>  	}
>  	}

-- Hannes