From: Mark Levedahl <mlevedahl@gmail.com>
Date: Sat, 16 May 2026 14:48:26 GMT
Subject: Re: [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree
Message-ID: <6c189058-18f0-47cf-a663-d4d177282961@gmail.com>
In-Reply-To: <4d25544d-1a7e-4407-9191-1fb05ff55244@kdbg.org>



On 5/16/26 4:14 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
>> 2010-01-23), has had the intent to allow starting from inside a
>> repository, then switching to the parent directory if that is a valid
>> worktree.
>>
>> This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
>> --show-toplevel without a worktree an error", 2019-11-19) in git, but
>> breaking this git-gui feature was unintentional.
>>
>> Add a proc to test if the parent of the git repository is a valid
>> worktree, and set that directory as the worktree if so. Use invocations
>> of git rev-parse to assure all validity and safety checks included in
>> git-core are executed.
> BTW, missing sign-off.
>
>> ---
>>  git-gui.sh | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index a03eaa7..e326401 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1100,6 +1100,23 @@ unset argv0dir
>>  ##
>>  ## repository setup
>>  
>> +proc is_parent_worktree {} {
>> +	# Directory 'parent' of a repository named 'parent/.git' might be the worktree
>> +	set ok 0
>> +	if {[file tail $::_gitdir] eq {.git}} {
>> +		set gitdir_parent [file join $::_gitdir {..}]
>> +		set expected_worktree [file normalize $gitdir_parent]
> We have [file dirname ...]. Is there a reason to avoid it?
>
>> +		catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
>> +		if {[string compare $expected_worktree $git_worktree] == 0} {
> The purpose of this check should be explained in a comment. I think it is:
>
> For a repository with the database in a directory named .git we assume
> that the working tree is the directory containing .git. But
> configuration may point to a different worktree. Then we do not want to
> hold on to our assumption.
>
> However, whether [git -C elsewhere ...] uses the same gitdir that we
> have discovered so far cannot be told from this piece of code alone.
> Therefore, I think it is wrong to extract this check into a function.
>
> Also, I don't think we can use string comparison here. On Windows, the
> command returns the Windows style path, but Tcl my operate with a POSIX
> style path.
As you have correctly inferred, am trying to unambiguously establish that git running in
the parent directory is using  the child .git as the repository. I think this actually
requires two calls to git-revparse (--absolute-git-dir and --show-toplevel).
- the current git repo is valid to support a worktree. Will rework.

>> +			set ::_prefix {}
>> +			set ::_gitworktree $git_worktree
>> +			cd $git_worktree
> So many side-effects in a function whose name suggests that it only does
> some checks. Please, don't do that.
>
>> +			set ok 1
>> +		}
>> +	}
>> +	return $ok
>> +}
>> +
>>  proc is_gitvars_error {err} {
>>  	set havevars 0
>>  	set GIT_DIR {}
> In general, I am not a fan of commits that add new functions, but no
> call sites. Please squash this into 10/11. Ditto for is_gitvars_error in
> 06/11.
>
> -- Hannes
>

Next round should address all of your comments.
Mark