From: Johannes Sixt <j6t@kdbg.org>
Date: Sun, 24 May 2026 07:00:30 GMT
Subject: Re: [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands
Message-ID: <03385a9e-bc94-4096-8fa5-7b8298269230@kdbg.org>
In-Reply-To: <20260520202411.108764-12-mlevedahl@gmail.com>

Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git-gui accepts subcommands blame | browser | citool, and assumes the
> subcommand is 'gui' if none is actually given, But, git-gui also has a
> repository picker (choose_repository::pick) that can create a new
> repository + worktree, or choose an existing one, switch to that, and
> the run the gui. The user has no direct control over invoking the
> picker, instead the picker is triggered by failure in the repository /
> worktree discovery process: this includes being started in a directory
> not controlled by git, which is probably the intended use case.
> 
> The picker can appear when the user has no intention of creating a new
> worktree, and the user cannot use the picker to create a new worktree
> inside another.
> 
> So, add two explicit subcommands:
>     gui  - Run the gui if repository/worktree discovery succeeds, or die
>            with an error message, but never run the picker.
>     pick - First run the picker, regardless, then start the gui in
>            the chosen worktree.
> 
> Nothing in this changes the prior behavior, the alternates above must be
> explicitly selected to see any change.

Good.

> @@ -1174,7 +1184,7 @@ proc unset_gitdir_vars {} {
>  
>  # find repository.
>  set _gitdir {}
> -if {$_gitdir eq {}} {
> +if {[is_enabled gitdir_discovery]} {

This makes a factually unconditional branch into a conditional one.

>  	if {[catch {
>  		set _gitdir [git rev-parse --absolute-git-dir]
>  	} err]} {
> @@ -1186,7 +1196,7 @@ if {$_gitdir eq {}} {
>  }
>  
>  set picked 0
> -if {$_gitdir eq {}} {
> +if {$_gitdir eq {} && [is_enabled picker]} {
>  	unset_gitdir_vars
>  	load_config 1
>  	apply_config
> @@ -1202,6 +1212,12 @@ if {$_gitdir eq {}} {
>  	set picked 1
>  }
>  
> +if {$_gitdir eq {}} {
> +	catch {wm withdraw .}
> +	error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]

I wondered where this $err is filled in, and it can only be the error
from a failed gitdir discovery. Good.

> +	exit 1
> +}
> +
>  # find worktree, continue without if not required
>  if {[catch {
>  	set _gitworktree [git rev-parse --show-toplevel]
This looks good!

-- Hannes