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

From: Mark Levedahl <mlevedahl@gmail.com>
Date: May 25, 2026, 16:02 UTC
Message-ID: <3cb066ca-0d1f-4197-ae96-050e94db2453@gmail.com>
In-Reply-To: <43f070e4-e624-4a33-8c24-294520fb503a@kdbg.org>

On 5/24/26 3:16 AM, Johannes Sixt wrote:

Show 9 quoted lines

> 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.

Show 10 quoted lines

>
> 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
>