From: Johannes Sixt <j6t@kdbg.org>
Date: Fri, 15 May 2026 15:59:37 GMT
Subject: Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
Message-ID: <7544deeb-163c-4444-833a-7b840a7caa4a@kdbg.org>
In-Reply-To: <20260514143322.865587-5-mlevedahl@gmail.com>

Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui includes a 'repository picker', which allows creating a new
> repository + worktree, or selecting a worktree from a recent list.
> git-gui runs the picker when a valid git repository is not found. All of
> the code for this is embedded in the discovery process block, making the
> latter more difficult to read, and also making things more difficult if
> we want to have an explicit 'pick' subcommand to force this to run.

OK, let's see how useful this becomes.

> 
> Let's move this invocation and supporting code to a separate proc,
> aiding in subsequent refactoring. Assure GIT_DIR and GIT_WORK_TREE are
> unset, configuration is loaded, ant that _gitdir is correctly set

s/ant/and/

> afterwards. As this is invoked before worktree discovery, later code
> will set that anyway so need not be included here.
> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index 387cad6..0b73c35 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1139,6 +1139,16 @@ proc unset_gitdir_vars {} {
>  }
>  
>  set picked 0
> +proc pick_repo {} {
> +	unset_gitdir_vars
> +	load_config 1
> +	apply_config
> +	choose_repository::pick
> +	set _gitdir [git rev-parse --absolute-git-dir]
> +	set _prefix {}
> +	set picked 1
> +}
> +

So, this isn't intended as a plain move of code? Since we set _gitdir
here, we could remove the corresonding lines from lib/choose_repository.tcl.

Is the variable "picked" only needed for this particular picker
invocation? Then it should not be set in the function, but at the call site.

>  if {[catch {
>  		set _gitdir $env(GIT_DIR)
>  		set _prefix {}
> @@ -1149,13 +1159,7 @@ if {[catch {
>  		set _gitdir [git rev-parse --git-dir]
>  		set _prefix [git rev-parse --show-prefix]
>  	} err]} {
> -	load_config 1
> -	apply_config
> -	choose_repository::pick
> -	if {![file isdirectory $_gitdir]} {
> -		exit 1
> -	}
> -	set picked 1
> +		pick_repo

The indentation is off here.

>  }
>  
>  # Use object format as hash algorithm (either "sha1" or "sha256")

-- Hannes