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>

diff --git a/git-gui.sh b/git-gui.sh
index e7a87fc9962edaf7867d34587af4d1f1a5d21e77..efa745dc48fb63a6795ba24797f7049c0d64f92c 100755 (executable)
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1153,9 +1153,14 @@ if {[catch {
        load_config 1
        apply_config
        choose_repository::pick
-       if {![file isdirectory $_gitdir]} {
+       if {[catch {
+                       set _gitdir [git rev-parse --git-dir]
+               } err]} {
+               catch {wm withdraw .}
+               error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] \n\n$err"]
                exit 1
        }
+       set _prefix {}
        set picked 1
 }
 

diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 7e1462a20c59d9b3a775ea1d57a7f868f507d674..4b06afee93f9a7e96cb7d1b049698cbce367bcc1 100644 (file)
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -15,7 +15,7 @@ field w_recentlist ; # Listbox containing recent repositories
 field w_localpath  ; # Entry widget bound to local_path
 
 field done              0 ; # Finished picking the repository?
-field clone_ok      false ; # clone succeeeded
+field pick_ok           0 ; # true if repo pick/clone succeeded
 field local_path       {} ; # Where this repository is locally
 field origin_url       {} ; # Where we are cloning from
 field origin_name  origin ; # What we shall call 'origin'
@@ -220,6 +220,8 @@ constructor pick {} {
        if {$top eq {.}} {
                eval destroy [winfo children $top]
        }
+
+       return $pick_ok
 }
 
 method _center {} {
@@ -327,8 +329,7 @@ method _git_init {} {
        }
 
        _append_recentrepos [pwd]
-       set ::_gitdir .git
-       set ::_prefix {}
+       set pick_ok 1
        return 1
 }
 
@@ -409,6 +410,7 @@ method _do_new2 {} {
        if {![_git_init $this]} {
                return
        }
+       set pick_ok 1
        set done 1
 }
 
@@ -621,7 +623,7 @@ method _do_clone2 {} {
        }
 
        tkwait variable @done
-       if {!$clone_ok} {
+       if {!$pick_ok} {
                error_popup [mc "Clone failed."]
                return
        }
@@ -632,18 +634,12 @@ method _do_clone2_done {ok} {
        if {$ok} {
                if {[catch {
                        cd $local_path
-                       set ::_gitdir .git
-                       set ::_prefix {}
                        _append_recentrepos [pwd]
                } err]} {
                        set ok 0
                }
        }
-       if {!$ok} {
-               set ::_gitdir {}
-               set ::_prefix {}
-       }
-       set clone_ok $ok
+       set pick_ok $ok
        set done 1
 }
 
@@ -721,8 +717,7 @@ method _do_open2 {} {
        }
 
        _append_recentrepos [pwd]
-       set ::_gitdir $actualgit
-       set ::_prefix {}
+       set pick_ok 1
        set done 1
 }