Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands

> On 5/16/26 4:18 AM, Johannes Sixt wrote:
>> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>>> diff --git a/git-gui.sh b/git-gui.sh
>>> index 3a83dd5..c56aeef 100755
>>> --- a/git-gui.sh
>>> +++ b/git-gui.sh
>>> @@ -1021,6 +1021,7 @@ proc load_config {include_global} {
>>>  ##
>>>  ## feature option selection
>>>  
>>> +set run_picker_on_error 1
>>>  if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
>>>  	unset _junk
>>>  } else {
>>> @@ -1030,6 +1031,7 @@ if {$subcommand eq {gui.sh}} {
>>>  	set subcommand gui
>>>  }
>>>  if {$subcommand eq {gui} && [llength $argv] > 0} {
>>> +	set run_picker_on_error 0
>>>  	set subcommand [lindex $argv 0]
>>>  	set argv [lrange $argv 1 end]
>>>  }
>>> @@ -1047,6 +1049,7 @@ blame {
>>>  	disable_option multicommit
>>>  	disable_option branch
>>>  	disable_option transport
>>> +	set run_picker_on_error 0
>>>  }
>>>  citool {
>>>  	enable_option singlecommit
>>> @@ -1055,6 +1058,7 @@ citool {
>>>  	disable_option multicommit
>>>  	disable_option branch
>>>  	disable_option transport
>>> +	set run_picker_on_error 0
>>>  
>>>  	while {[llength $argv] > 0} {
>>>  		set a [lindex $argv 0]
>> Can we please use the available disable_option and enable_option feature
>> instead of a new variable. Just for consistency around repository discovery.
>>
>>> @@ -1162,14 +1166,28 @@ proc pick_repo {} {
>>>  	set picked 1
>>>  }
>>>  
>>> +# run repository picker if explicitly requested
>>> +switch -- $subcommand {
>>> +	pick {
>>> +		pick_repo
>>> +		set subcommand gui
>>> +		set run_picker_on_error 0
>>> +	}
>>> +}
>>> +
>> It just feels wrong to have a new pick_repo call before repository
>> discovery. Can we not treat this case below as if regular repository
>> discovery failed and then end up in the existing call of pick_repo?
> 
> So, your suggestion is to create an error inside the catch clause, assure GIT_VAR and
> GIT_WORK_TREE are unset so we don't throw and error message and abort, and then fall
> through to the existing pick_repo clause?