Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- May 16, 2026, 08:18 UTC
- Message-ID
- <e336544b-941d-43ed-890f-2b8950dbaf88@kdbg.org>
- In-Reply-To
- <20260514143322.865587-12-mlevedahl@gmail.com>
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
Show 21 quoted lines
> 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 discover 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 new 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.
OK.
Show 42 quoted lines
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> 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.
Show 13 quoted lines
> @@ -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?
Show 26 quoted lines
> # find repository.
> if {[catch {
> set _gitdir [git rev-parse --absolute-git-dir]
> } err]} {
> if {[is_gitvars_error $err]} {
> exit 1
> - } else {
> + }
> + if {$run_picker_on_error} {
> pick_repo
> + } else {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
> + exit 1
> }
> }
>
> @@ -3051,7 +3069,7 @@ gui {
> # fall through to setup UI for commits
> }
> default {
> - set err "[mc usage:] $argv0 \[{blame|browser|citool}\]"
> + set err "[mc usage:] $argv0 \[{blame|browser|citool|gui|pick}\]"
> if {[tk windowingsystem] eq "win32"} {
> wm withdraw .
> tk_messageBox -icon error -message $err \We don't need to switch on the new subcommands?
As a follow-up to my comment on 04/11: How relevant is it that variable $picked is set in a 'git gui pick' invocation?
-- Hannes