Re: [PATCH v1 10/11] git-gui: improve worktree discovery
- From
Mark Levedahl <mlevedahl@gmail.com>
- Date
- May 19, 2026, 19:00 UTC
- 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:
Show 26 quoted lines
> 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