From: Johannes Sixt <j6t@kdbg•org>
To: Mark Levedahl <mlevedahl@gmail•com>
Cc: egg_mushroomcow@foxmail•com, bootaina702@gmail•com, git@vger•kernel.org
Subject: Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
Date: Sat, 23 May 2026 22:31:37 +0200 [thread overview]
Message-ID: <8b4e8b73-eb2c-41bb-9653-08e38fedd434@kdbg.org> (raw)
In-Reply-To: <20260520202411.108764-11-mlevedahl@gmail.com>
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> 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`.
>
> 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.
>
> 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.
>
> 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'.
>
> 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.
> - 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
next prev parent reply other threads:[~2026-05-23 20:31 UTC|newest]
Thread overview: 134+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 16:28 [PATCH] git-gui: handle bare repo or missing worktree Shroom Moo
2026-04-29 6:58 ` Johannes Sixt
2026-04-29 17:32 ` [PATCH v2 1/1] git-gui: protect rev-parse --show-toplevel call Shroom Moo
2026-04-29 20:14 ` Mark Levedahl
2026-04-30 10:02 ` [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir Shroom Moo
2026-04-30 16:18 ` Mark Levedahl
2026-05-01 10:22 ` [PATCH v3 1/1] git-gui: handle missing worktree and separated Shroom Moo
2026-05-01 13:13 ` [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir Johannes Sixt
2026-05-01 16:42 ` Mark Levedahl
2026-05-02 21:51 ` Mark Levedahl
2026-05-03 8:53 ` Johannes Sixt
2026-05-04 15:13 ` Mark Levedahl
2026-05-05 3:40 ` Mark Levedahl
2026-05-06 7:32 ` Johannes Sixt
2026-05-06 11:27 ` Mark Levedahl
2026-05-06 12:57 ` Johannes Sixt
2026-05-06 14:05 ` Mark Levedahl
2026-05-07 5:09 ` Mark Levedahl
2026-05-01 10:54 ` [PATCH v4 " Shroom Moo
2026-05-04 14:59 ` [PATCH v5 1/1] git-gui: restructure repository startup Shroom Moo
2026-05-06 7:15 ` Johannes Sixt
2026-05-06 20:27 ` [PATCH v6 0/3] git-gui: robustify startup and fix environment handling Shroom Moo
2026-05-09 13:37 ` [PATCH v7 " Shroom Moo
2026-05-14 14:28 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
2026-05-15 15:54 ` Johannes Sixt
2026-05-16 13:38 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing Mark Levedahl
2026-05-15 15:56 ` Johannes Sixt
2026-05-16 14:21 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
2026-05-15 15:58 ` Johannes Sixt
2026-05-16 14:25 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc Mark Levedahl
2026-05-15 11:00 ` Aina Boot
2026-05-15 13:33 ` Mark Levedahl
2026-05-15 15:59 ` Johannes Sixt
2026-05-16 14:29 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 05/11] git-gui: use --absolute-git-dir Mark Levedahl
2026-05-15 16:00 ` Johannes Sixt
2026-05-16 14:33 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 06/11] git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
2026-05-15 16:06 ` Johannes Sixt
2026-05-16 14:38 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
2026-05-16 8:12 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree Mark Levedahl
2026-05-16 8:14 ` Johannes Sixt
2026-05-16 14:48 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 10/11] git-gui: improve worktree discovery Mark Levedahl
2026-05-16 8:16 ` Johannes Sixt
2026-05-16 15:28 ` Mark Levedahl
2026-05-19 8:16 ` Johannes Sixt
2026-05-19 19:00 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
2026-05-16 8:18 ` Johannes Sixt
2026-05-16 15:42 ` Mark Levedahl
2026-05-19 8:21 ` Johannes Sixt
2026-05-19 18:45 ` Mark Levedahl
2026-05-19 21:15 ` Johannes Sixt
2026-05-16 8:28 ` [PATCH v1 00/11] Improve git gui operation without a worktree Johannes Sixt
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
2026-05-22 8:06 ` Johannes Sixt
2026-05-22 11:54 ` Mark Levedahl
2026-05-23 8:18 ` Johannes Sixt
2026-05-23 11:46 ` Aina Boot
2026-05-23 16:08 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 02/11] git-gui: return status from choose_repository::pick Mark Levedahl
2026-05-22 8:18 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 03/11] git-gui: use --absolute-git-dir Mark Levedahl
2026-05-22 8:25 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
2026-05-22 8:46 ` Johannes Sixt
2026-05-22 12:04 ` Mark Levedahl
2026-05-22 23:00 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 05/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery Mark Levedahl
2026-05-23 13:26 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir Mark Levedahl
2026-05-21 4:55 ` Shroom Moo
2026-05-21 17:45 ` Mark Levedahl
2026-05-22 15:09 ` Shroom Moo
2026-05-22 16:57 ` Mark Levedahl
2026-05-23 8:01 ` Johannes Sixt
2026-05-23 11:47 ` Shroom Moo
2026-05-23 14:06 ` Johannes Sixt
2026-05-23 15:33 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix) Mark Levedahl
2026-05-23 14:08 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
2026-05-23 14:23 ` Johannes Sixt
2026-05-23 15:43 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation Mark Levedahl
2026-05-21 5:02 ` Shroom Moo
2026-05-21 17:35 ` Mark Levedahl
2026-05-22 15:05 ` Shroom Moo
2026-05-23 18:19 ` [PATCH] fixup git-gui: allow blame to show uncommitted changes Mark Levedahl
2026-05-23 20:31 ` Johannes Sixt [this message]
2026-05-20 20:24 ` [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
2026-05-24 7:00 ` Johannes Sixt
2026-05-24 7:16 ` [PATCH v2 00/11] Improve git gui operation without a worktree Johannes Sixt
2026-05-25 16:02 ` Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 00/12] " Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 01/12] git-gui: use HEAD as current branch when detached Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 02/12] git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 03/12] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 04/12] git-gui: do not change global vars in choose_repository::pick Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 05/12] git-gui: use --absolute-git-dir Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 06/12] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 07/12] git-gui: use git rev-parse for worktree discovery Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 08/12] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 09/12] git-gui: try harder to find worktree from gitdir Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 10/12] git-gui: allow specifying path '.' to the browser Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 11/12] git-gui: check browser/blame arguments carefully Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 12/12] git-gui: add gui and pick as explicit subcommands Mark Levedahl
2026-06-02 17:34 ` [PATCH v3 00/12] Improve git gui operation without a worktree Johannes Sixt
2026-06-02 18:54 ` Mark Levedahl
2026-06-02 21:05 ` Johannes Sixt
[not found] ` <20260509133756.1367-1-egg_mushroomcow@foxmail.com>
2026-05-09 13:37 ` [PATCH v7 1/3] git-gui: restructure repository startup Shroom Moo
2026-05-15 8:26 ` Johannes Sixt
2026-05-09 13:37 ` [PATCH v7 2/3] git-gui: disable gitk visualization when no worktree available Shroom Moo
2026-05-15 8:28 ` Johannes Sixt
2026-05-09 13:37 ` [PATCH v7 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early Shroom Moo
2026-05-15 8:28 ` Johannes Sixt
[not found] ` <20260506202751.3294-1-egg_mushroomcow@foxmail.com>
2026-05-06 20:27 ` [PATCH v6 1/3] git-gui: restructure repository startup Shroom Moo
2026-05-06 20:27 ` [PATCH v6 2/3] git-gui: disable gitk visualization when no worktree available Shroom Moo
2026-05-06 20:27 ` [PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early Shroom Moo
2026-05-07 15:50 ` Mark Levedahl
2026-05-09 8:46 ` Aina Boot
2026-05-09 9:55 ` Shroom Moo
2026-04-29 18:28 ` [PATCH] git-gui: handle bare repo or missing worktree Shroom Moo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8b4e8b73-eb2c-41bb-9653-08e38fedd434@kdbg.org \
--to=j6t@kdbg$(echo .)org \
--cc=bootaina702@gmail$(echo .)com \
--cc=egg_mushroomcow@foxmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=mlevedahl@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox