From: Mark Levedahl <mlevedahl@gmail.com>
Date: Tue, 19 May 2026 19:00:21 GMT
Subject: Re: [PATCH v1 10/11] git-gui: improve worktree discovery
Message-ID: <7851b418-0641-4365-aa03-4ee4d95509ea@gmail.com>
In-Reply-To: <5081fcc5-19b5-49aa-a33c-2c13aba7edb1@kdbg.org>



On 5/19/26 4:16 AM, Johannes Sixt wrote:
> Am 16.05.26 um 17:28 schrieb Mark Levedahl:
>> On 5/16/26 4:16 AM, Johannes Sixt wrote:
>>> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>>>> +	if {[is_gitvars_error $err]} {
>>>> +		exit 1
>>>> +	}
>>>> +	set _gitworktree {}
>>>> +	set _prefix {}
>>>> +	if {[is_enabled bare]} {
>>>> +		cd $_gitdir
>>> Why change the directory here? If we run `git gui browser master dir` we
>>> do not want to change the directory in an uncontrolled manner. The
>>> argument parser will want to check for the existence of files, and then
>>> we do not want to operate from a random directory.
>>>
>>> Also, I think that the check must be for [is_bare] and not [is_enabled
>>> bare].
>> [is_enabled_bare] is correct. This code handles the case: 
>>     - neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
>> is currently true.
>>     - the command given is browser or blame so a worktree is not needed. We can proceed.
> But in the case where the command is browser or blame, the argument
> parser must later check for the existence of files, provided that a
> worktree is present. But this conditional would change directory to
> somewhere that is not a worktree at all even though a worktree is
> available. So, I am still convinced that [is_bare] is correct.
I did change this.   but...
I have reworked the blame/browser parser so it fully matches git blame parsing for the
single rev + path (or path rev) cases, all now do the same thing with or without a
worktree as they all work from git history, so having a worktree becomes almost moot (I
found issues with how git-gui handles --, it is dead wrong in some cases if the intent is
to match git-blame). What doesn't change is what blame displays based upon uncommitted or
staged changes in the worktree (if it does anything, but comments from 2007 suggest it
does, I haven't tested). I've done nothing to change that, only finding the args to pass
in to browser / blame. So, if a worktree exists, blame will still use the info there as it
does now.

Mark