From: Mark Levedahl <mlevedahl@gmail.com>
Date: Sat, 16 May 2026 14:29:44 GMT
Subject: Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
Message-ID: <d56431e8-79b5-428d-9bfa-e814e41af3c5@gmail.com>
In-Reply-To: <7544deeb-163c-4444-833a-7b840a7caa4a@kdbg.org>



On 5/15/26 11:59 AM, Johannes Sixt wrote:
> 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/
will fix
>> 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.
it should be a plain move.
> 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.
I need to better understand how "picked" is used to decide... will do before an update.
>>  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.
will fix.

Mark