Subject: Re: [PATCH v2 00/11] Improve git gui operation without a worktree

From: Mark Levedahl <[email-masked]>
Date: Mon, 25 May 2026 12:02:10 -0400

On 5/24/26 3:16 AM, Johannes Sixt wrote:
> Am 20.05.26 um 22:23 schrieb Mark Levedahl:
> I've completed my review of this iteration.
>
> Repository and working tree discovery is already converging fast.
> However, I have issues with the proposed argument parsing of the browser
> and blame modes, in particular, I don't think that we need to
> accommodate the uncanny file-before-rev argument order and that it
> disregards the worktree completely. Maybe we should postpone any changes
> in this area, if possible?
I'm not willing to give up on browser/blame yet: making these work without a worktree was
my motivator to start this series. Ignoring implementation details, etc., the issues I see
here are:

The undocumented feature to accept rev / path or path / rev, as does git blame. The latter
at least has a comment on what is expected in the code, but no mention of this exists in
blames man-page, nor that of any other git command I've examined. I'd prefer to just
remove it, I'll take your comment above as agreement in principal.

browser and blame are both fundamentally about git history. Considering browser.tcl and
blame.tcl, which produce the displays:

browser shows only content from a tree in a commit. It never uses any information from a
worktree. Given a non-existent path (or a non-existent rev),
browser displays an empty window, not an error message.

blame can take content from a file in a commit, or from the worktree as long as the file
is in a commit. A modified file in the worktree has changed / added lines annotated as
uncommitted work. But, given a file not in rev, blame displays the file with no
annotations at all, not as uncommitted work, and no error message.

So. both blame and browser require that $path is contained in $rev, even if $rev defaults
to HEAD.
The parser never checks this, though.

These commands, that *should* work fine without a worktree do not. and display confusing
information (e.g., a blank browser window, or unannotated file) when a simple error
message from the parser would convey more information.

>
> Throughout, we use a strange indentation style of 'if {[catch ...' that
> is violated in new code, but I left uncommented. It should indent the
> catch body one additional level like so:
>
> 	if {catch {
> 			commands that can fail
> 		} err]} {
> 		error handling here
> 	}
Yes, just count the number of { - number of }. Vim's indent mode for tcl gets this very
wrong. All fixed, I hope.
>
> Thank you very much for working on this topic.
and thank you for the very thorough review.
> -- Hannes