@matheusbsbs / matheus-bagliano / commits / 587a4ac448

git-gui: do not change global vars in choose_repository::pick

The repository picker (choose_repository::pick, AKA pick) on success always returns with the current directory at the root of the selected worktree, with the global variable _gitdir holding the name of the git repository, possibly as a relative path, and _prefix {}. The worktree root (_gitworktree) is not filled out, and if the selection was from the "recent" list, no validation has occurred beyond testing that the worktree root exists. So, repository and worktree validation are still needed to be sure the new repo + worktree is usable. pick only supports worktrees with a .git entry in the worktree root, so git repository and worktree discovery will work starting in the current directory on return. In cases of error, or user abort, pick exits the process rather than returning. So, let's change pick to not alter any global values, with success indicated by the process returning to the caller. In this case, the current directory is the worktree root, with a .git entry. The caller then proceeds with normal discovery to find and validate both repository and worktree. With this, pick now returns 1 in the success case, but additional work would be necessary to return from conditions where 0 should be returned. Checking this return value would be superfluous. Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Mark Levedahl committed May 31, 2026 at 19:02 UTC 587a4ac448e3c0393f78770c48e85b5e90597470
2 files changed +14 -14
git-gui.sh
+6 -1
@@ -1153,9 +1153,14 @@ if {[catch {
1153 load_config 1
1154 apply_config
1155 choose_repository::pick
1156 - if {![file isdirectory $_gitdir]} {
1156 + if {[catch {
1157 + set _gitdir [git rev-parse --git-dir]
1158 + } err]} {
1159 + catch {wm withdraw .}
1160 + error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] \n\n$err"]
1161 exit 1
1162 }
1163 + set _prefix {}
1164 set picked 1
1165 }
1166
lib/choose_repository.tcl
+8 -13
@@ -15,7 +15,7 @@ field w_recentlist ; # Listbox containing recent repositories
15 field w_localpath ; # Entry widget bound to local_path
16
17 field done 0 ; # Finished picking the repository?
18 -field clone_ok false ; # clone succeeeded
18 +field pick_ok 0 ; # true if repo pick/clone succeeded
19 field local_path {} ; # Where this repository is locally
20 field origin_url {} ; # Where we are cloning from
21 field origin_name origin ; # What we shall call 'origin'
@@ -220,6 +220,8 @@ constructor pick {} {
220 if {$top eq {.}} {
221 eval destroy [winfo children $top]
222 }
223 +
224 + return $pick_ok
225 }
226
227 method _center {} {
@@ -327,8 +329,7 @@ method _git_init {} {
329 }
330
331 _append_recentrepos [pwd]
330 - set ::_gitdir .git
331 - set ::_prefix {}
332 + set pick_ok 1
333 return 1
334 }
335
@@ -409,6 +410,7 @@ method _do_new2 {} {
410 if {![_git_init $this]} {
411 return
412 }
413 + set pick_ok 1
414 set done 1
415 }
416
@@ -621,7 +623,7 @@ method _do_clone2 {} {
623 }
624
625 tkwait variable @done
624 - if {!$clone_ok} {
626 + if {!$pick_ok} {
627 error_popup [mc "Clone failed."]
628 return
629 }
@@ -632,18 +634,12 @@ method _do_clone2_done {ok} {
634 if {$ok} {
635 if {[catch {
636 cd $local_path
635 - set ::_gitdir .git
636 - set ::_prefix {}
637 _append_recentrepos [pwd]
638 } err]} {
639 set ok 0
640 }
641 }
642 - if {!$ok} {
643 - set ::_gitdir {}
644 - set ::_prefix {}
645 - }
646 - set clone_ok $ok
642 + set pick_ok $ok
643 set done 1
644 }
645
@@ -721,8 +717,7 @@ method _do_open2 {} {
717 }
718
719 _append_recentrepos [pwd]
724 - set ::_gitdir $actualgit
725 - set ::_prefix {}
720 + set pick_ok 1
721 set done 1
722 }
723