* [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-15 15:54 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing Mark Levedahl
` (11 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
Invoking "git-gui browser rev ." should show the file browser for the
commitish rev, starting at the root directory. This errors out in
normalize_relpath because the '.' is removed, yielding an empty list as
argument to [file join ...]. Fix this.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/git-gui.sh b/git-gui.sh
index 23fe76e..6048f92 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2965,7 +2965,11 @@ proc normalize_relpath {path} {
}
lappend elements $item
}
- return [eval file join $elements]
+ if {$elements ne {}} {
+ return [eval file join $elements]
+ } else {
+ return {}
+ }
}
# -- Not a normal commit type invocation? Do that instead!
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser
2026-05-14 14:33 ` [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
@ 2026-05-15 15:54 ` Johannes Sixt
2026-05-16 13:38 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-15 15:54 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> Invoking "git-gui browser rev ." should show the file browser for the
> commitish rev, starting at the root directory. This errors out in
> normalize_relpath because the '.' is removed, yielding an empty list as
> argument to [file join ...]. Fix this.
Good catch!
The description isn't precise, though. '.' means to list the current
directory. The mentioned problem happens only if this is also the root
of the working tree.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 23fe76e..6048f92 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2965,7 +2965,11 @@ proc normalize_relpath {path} {
> }
> lappend elements $item
> }
> - return [eval file join $elements]
> + if {$elements ne {}} {
> + return [eval file join $elements]
> + } else {
> + return {}
> + }
> }
>
> # -- Not a normal commit type invocation? Do that instead!
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-15 15:56 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
` (10 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git-gui has subcommands blame and browser, both of which accept a
pathname, possibly preceded by a commit-ish item to specify a revision.
Also, blame can take a first argument that gives a line number to focus.
The command line parser for the above is more complex than needed, and
cannot work without a worktree as the pathname objects are checked
against the current worktree for existence. This also precludes naming a
directory or file that does not exist on the currently checked out
branch.
So, replace this with a simpler parser that looks at argument number and
number of arguments to know what value to expect. The blame and browser
backends already have error checking with diagnostic information, so
defer most error checking to those. Also, allow a line-number selection
to be given and silently ignored for the browser, further simplifying
this code.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 66 +++++++++++++-----------------------------------------
1 file changed, 16 insertions(+), 50 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 6048f92..a951fcd 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2986,51 +2986,34 @@ blame {
set head {}
set path {}
set jump_spec {}
- set is_path 0
+ set nargs [llength $argv]
+ if {$nargs < 1} {
+ usage
+ }
+ set argn 0
foreach a $argv {
- set p [file join $_prefix $a]
+ set argn [expr {$argn + 1}]
- if {$is_path || [file exists $p]} {
- if {$path ne {}} usage
- set path [normalize_relpath $p]
- break
- } elseif {$a eq {--}} {
- if {$path ne {}} {
- if {$head ne {}} usage
- set head $path
- set path {}
+ if {$argn < $nargs} {
+ # revision or line number
+ if {[regexp {^--line=(\d+)$} $a a lnum]} {
+ set jump_spec [list $lnum]
+ } else {
+ set head $a
}
- set is_path 1
- } elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
- if {$jump_spec ne {} || $head ne {}} usage
- set jump_spec [list $lnum]
- } elseif {$head eq {}} {
- if {$head ne {}} usage
- set head $a
- set is_path 1
- } else {
- usage
- }
- }
- unset is_path
-
- if {$head ne {} && $path eq {}} {
- if {[string index $head 0] eq {/}} {
- set path [normalize_relpath $head]
- set head {}
} else {
- set path [normalize_relpath $_prefix$head]
- set head {}
+ set path [normalize_relpath $a]
}
}
if {$head eq {}} {
load_current_branch
+ set head $current_branch
} else {
if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
if {[catch {
- set head [git rev-parse --verify $head]
- } err]} {
+ set head [git rev-parse --verify $head]
+ } err]} {
if {[tk windowingsystem] eq "win32"} {
tk_messageBox -icon error -title [mc Error] -message $err
} else {
@@ -3046,26 +3029,9 @@ blame {
switch -- $subcommand {
browser {
if {$jump_spec ne {}} usage
- if {$head eq {}} {
- if {$path ne {} && [file isdirectory $path]} {
- set head $current_branch
- } else {
- set head $path
- set path {}
- }
- }
browser::new $head $path
}
blame {
- if {$head eq {} && ![file exists $path]} {
- catch {wm withdraw .}
- tk_messageBox \
- -icon error \
- -type ok \
- -title [mc "git-gui: fatal error"] \
- -message [mc "fatal: cannot stat path %s: No such file or directory" $path]
- exit 1
- }
blame::new $head $path $jump_spec
}
}
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing
2026-05-14 14:33 ` [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing Mark Levedahl
@ 2026-05-15 15:56 ` Johannes Sixt
2026-05-16 14:21 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-15 15:56 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui has subcommands blame and browser, both of which accept a
> pathname, possibly preceded by a commit-ish item to specify a revision.
> Also, blame can take a first argument that gives a line number to focus.
>
> The command line parser for the above is more complex than needed, and
> cannot work without a worktree as the pathname objects are checked
> against the current worktree for existence. This also precludes naming a
> directory or file that does not exist on the currently checked out
> branch.
While the old browser isn't simple, it implements the strategy "revs
before paths, no revs after the first path or '--'" that is applied by
every git command. The rewritten parser is only slightly simpler. It
should not ignore "--". Furthermore, the old parser ignored excessive
trailing arguments, while the new parser ignores excessive leading
arguments. Neither is desirable, and we should report an incorrect
argument list (if we don't, then I prefer the old behavior).
>
> So, replace this with a simpler parser that looks at argument number and
> number of arguments to know what value to expect. The blame and browser
> backends already have error checking with diagnostic information, so
> defer most error checking to those. Also, allow a line-number selection
> to be given and silently ignored for the browser, further simplifying
> this code.
The line number selection isn't ignored by the browser, but reported as
an incorrect usage before and after this patch.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 66 +++++++++++++-----------------------------------------
> 1 file changed, 16 insertions(+), 50 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 6048f92..a951fcd 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2986,51 +2986,34 @@ blame {
> set head {}
> set path {}
> set jump_spec {}
> - set is_path 0
> + set nargs [llength $argv]
> + if {$nargs < 1} {
> + usage
> + }
> + set argn 0
> foreach a $argv {
> - set p [file join $_prefix $a]
> + set argn [expr {$argn + 1}]
>
> - if {$is_path || [file exists $p]} {
> - if {$path ne {}} usage
> - set path [normalize_relpath $p]
> - break
> - } elseif {$a eq {--}} {
> - if {$path ne {}} {
> - if {$head ne {}} usage
> - set head $path
> - set path {}
> + if {$argn < $nargs} {
> + # revision or line number
> + if {[regexp {^--line=(\d+)$} $a a lnum]} {
> + set jump_spec [list $lnum]
> + } else {
> + set head $a
> }
> - set is_path 1
> - } elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
> - if {$jump_spec ne {} || $head ne {}} usage
> - set jump_spec [list $lnum]
> - } elseif {$head eq {}} {
> - if {$head ne {}} usage
> - set head $a
> - set is_path 1
> - } else {
> - usage
> - }
> - }
> - unset is_path
> -
> - if {$head ne {} && $path eq {}} {
> - if {[string index $head 0] eq {/}} {
> - set path [normalize_relpath $head]
> - set head {}
> } else {
> - set path [normalize_relpath $_prefix$head]
> - set head {}
> + set path [normalize_relpath $a]
This loses the capability to request a browser relative to a
subdirectory of the working tree. For example, "git gui browser main ."
now shows the top-level directory instead of the subdirectory when
invoked from a subdirectory.
> }
> }
>
> if {$head eq {}} {
> load_current_branch
> + set head $current_branch
> } else {
> if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
> if {[catch {
> - set head [git rev-parse --verify $head]
> - } err]} {
> + set head [git rev-parse --verify $head]
> + } err]} {
Please leave the indentation unchanged.
> if {[tk windowingsystem] eq "win32"} {
> tk_messageBox -icon error -title [mc Error] -message $err
> } else {
> @@ -3046,26 +3029,9 @@ blame {
> switch -- $subcommand {
> browser {
> if {$jump_spec ne {}} usage
> - if {$head eq {}} {
> - if {$path ne {} && [file isdirectory $path]} {
> - set head $current_branch
> - } else {
> - set head $path
> - set path {}
> - }
> - }
> browser::new $head $path
> }
> blame {
> - if {$head eq {} && ![file exists $path]} {
> - catch {wm withdraw .}
> - tk_messageBox \
> - -icon error \
> - -type ok \
> - -title [mc "git-gui: fatal error"] \
> - -message [mc "fatal: cannot stat path %s: No such file or directory" $path]
> - exit 1
> - }
> blame::new $head $path $jump_spec
> }
> }
The check for the existence of files is actually necessary to
disambiguate the meaning of the argument. If a file "maint" exists, then
the argument is to be interpreted as path, not as the ref "maint", even
if that exists, too.
I suggest to protect the "file exists" calls with ($_gitworktree ne {}
&& ...) or (![is_bare] && ...) to handle being invoked from a bare
repository. That is, in a bare repository we treat arguments the same as
files that do not exist in the currently checked-out branch.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing
2026-05-15 15:56 ` Johannes Sixt
@ 2026-05-16 14:21 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 14:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/15/26 11:56 AM, Johannes Sixt wrote:
snip...
> The check for the existence of files is actually necessary to
> disambiguate the meaning of the argument. If a file "maint" exists, then
> the argument is to be interpreted as path, not as the ref "maint", even
> if that exists, too.
>
> I suggest to protect the "file exists" calls with ($_gitworktree ne {}
> && ...) or (![is_bare] && ...) to handle being invoked from a bare
> repository. That is, in a bare repository we treat arguments the same as
> files that do not exist in the currently checked-out branch.
Let me start over, addressing only the use-case in a bare repository.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 01/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 02/11] git-gui: refactor browser / blame argument parsing Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-15 15:58 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc Mark Levedahl
` (9 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
environment, and furthmore unconditionally unsets these in many places.
But, GIT_WORK_TREE should be set only if it is not {} as the empty
value, really meaning no work-tree is found, causes git to throw fatal
errors (git-gui gets the error from branch --show-current). Fixing this
is required to allow blame and browser to operate from a repository
without a worktree.
Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
the environment, avoiding any error if they do not exist. Also, add a
function to export these, but export GIT_WORK_TREE only if not empty.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index a951fcd..387cad6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1122,6 +1122,22 @@ unset argv0dir
##
## repository setup
+proc set_gitdir_vars {} {
+ global _gitdir _gitworktree env
+ if {$_gitdir ne {}} {
+ set env(GIT_DIR) $_gitdir
+ }
+ if {$_gitworktree ne {}} {
+ set env(GIT_WORK_TREE) $_gitworktree
+ }
+}
+
+proc unset_gitdir_vars {} {
+ global env
+ catch {unset env(GIT_DIR)}
+ catch {unset env(GIT_WORK_TREE)}
+}
+
set picked 0
if {[catch {
set _gitdir $env(GIT_DIR)
@@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end]
}
-set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
+# Export the final paths
+set_gitdir_vars
######################################################################
##
@@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
# TODO we could make life easier (start up faster?) for gitk
# by setting these to the appropriate values to allow gitk
# to skip the heuristics to find their proper value
- unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
+ unset_gitdir_vars
}
safe_exec_bg [concat $cmd $revs "--" "--"]
- set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
+ set_gitdir_vars
cd $pwd
if {[info exists main_status]} {
@@ -2084,16 +2098,14 @@ proc do_git_gui {} {
# see note in do_gitk about unsetting these vars when
# running tools in a submodule
- unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
+ unset_gitdir_vars
set pwd [pwd]
cd $current_diff_path
safe_exec_bg [concat $exe gui]
- set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
+ set_gitdir_vars
cd $pwd
set status_operation [$::main_status \
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
2026-05-14 14:33 ` [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
@ 2026-05-15 15:58 ` Johannes Sixt
2026-05-16 14:25 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-15 15:58 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
> environment, and furthmore unconditionally unsets these in many places.
> But, GIT_WORK_TREE should be set only if it is not {} as the empty
> value, really meaning no work-tree is found, causes git to throw fatal
> errors (git-gui gets the error from branch --show-current). Fixing this
> is required to allow blame and browser to operate from a repository
> without a worktree.
>
> Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
> the environment, avoiding any error if they do not exist. Also, add a
> function to export these, but export GIT_WORK_TREE only if not empty.
Good. But as I said in a parallel thread, I actually concur with your
assessment in the coverletter of this patch series that GIT_WORK_TREE
should be not set at all. At least in the modes that require a working tree.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index a951fcd..387cad6 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1122,6 +1122,22 @@ unset argv0dir
> ##
> ## repository setup
>
> +proc set_gitdir_vars {} {
> + global _gitdir _gitworktree env
> + if {$_gitdir ne {}} {
> + set env(GIT_DIR) $_gitdir
> + }
> + if {$_gitworktree ne {}} {
> + set env(GIT_WORK_TREE) $_gitworktree
> + }
> +}
> +
> +proc unset_gitdir_vars {} {
> + global env
> + catch {unset env(GIT_DIR)}
> + catch {unset env(GIT_WORK_TREE)}
> +}
> +
> set picked 0
> if {[catch {
> set _gitdir $env(GIT_DIR)
> @@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end]
> }
>
> -set env(GIT_DIR) $_gitdir
> -set env(GIT_WORK_TREE) $_gitworktree
> +# Export the final paths
> +set_gitdir_vars
>
> ######################################################################
> ##
> @@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
> # TODO we could make life easier (start up faster?) for gitk
> # by setting these to the appropriate values to allow gitk
> # to skip the heuristics to find their proper value
> - unset env(GIT_DIR)
> - unset env(GIT_WORK_TREE)
> + unset_gitdir_vars
> }
> safe_exec_bg [concat $cmd $revs "--" "--"]
>
> - set env(GIT_DIR) $_gitdir
> - set env(GIT_WORK_TREE) $_gitworktree
> + set_gitdir_vars
> cd $pwd
>
> if {[info exists main_status]} {
> @@ -2084,16 +2098,14 @@ proc do_git_gui {} {
>
> # see note in do_gitk about unsetting these vars when
> # running tools in a submodule
> - unset env(GIT_DIR)
> - unset env(GIT_WORK_TREE)
> + unset_gitdir_vars
>
> set pwd [pwd]
> cd $current_diff_path
>
> safe_exec_bg [concat $exe gui]
>
> - set env(GIT_DIR) $_gitdir
> - set env(GIT_WORK_TREE) $_gitworktree
> + set_gitdir_vars
> cd $pwd
>
> set status_operation [$::main_status \
After these changes, a 'global env' probably becomes stale and could be
removed.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
2026-05-15 15:58 ` Johannes Sixt
@ 2026-05-16 14:25 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 14:25 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/15/26 11:58 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui unconditionally exports GIT_DIR and GIT_WORK_TREE to the
>> environment, and furthmore unconditionally unsets these in many places.
>> But, GIT_WORK_TREE should be set only if it is not {} as the empty
>> value, really meaning no work-tree is found, causes git to throw fatal
>> errors (git-gui gets the error from branch --show-current). Fixing this
>> is required to allow blame and browser to operate from a repository
>> without a worktree.
>>
>> Establish a pair of functions to remove GIT_DIR and GIT_WORK_TREE from
>> the environment, avoiding any error if they do not exist. Also, add a
>> function to export these, but export GIT_WORK_TREE only if not empty.
> Good. But as I said in a parallel thread, I actually concur with your
> assessment in the coverletter of this patch series that GIT_WORK_TREE
> should be not set at all. At least in the modes that require a working tree.
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>> ---
>> git-gui.sh | 32 ++++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index a951fcd..387cad6 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1122,6 +1122,22 @@ unset argv0dir
>> ##
>> ## repository setup
>>
>> +proc set_gitdir_vars {} {
>> + global _gitdir _gitworktree env
>> + if {$_gitdir ne {}} {
>> + set env(GIT_DIR) $_gitdir
>> + }
>> + if {$_gitworktree ne {}} {
>> + set env(GIT_WORK_TREE) $_gitworktree
>> + }
>> +}
>> +
>> +proc unset_gitdir_vars {} {
>> + global env
>> + catch {unset env(GIT_DIR)}
>> + catch {unset env(GIT_WORK_TREE)}
>> +}
>> +
>> set picked 0
>> if {[catch {
>> set _gitdir $env(GIT_DIR)
>> @@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
>> set _reponame [lindex $_reponame end]
>> }
>>
>> -set env(GIT_DIR) $_gitdir
>> -set env(GIT_WORK_TREE) $_gitworktree
>> +# Export the final paths
>> +set_gitdir_vars
>>
>> ######################################################################
>> ##
>> @@ -2050,13 +2066,11 @@ proc do_gitk {revs {is_submodule false}} {
>> # TODO we could make life easier (start up faster?) for gitk
>> # by setting these to the appropriate values to allow gitk
>> # to skip the heuristics to find their proper value
>> - unset env(GIT_DIR)
>> - unset env(GIT_WORK_TREE)
>> + unset_gitdir_vars
>> }
>> safe_exec_bg [concat $cmd $revs "--" "--"]
>>
>> - set env(GIT_DIR) $_gitdir
>> - set env(GIT_WORK_TREE) $_gitworktree
>> + set_gitdir_vars
>> cd $pwd
>>
>> if {[info exists main_status]} {
>> @@ -2084,16 +2098,14 @@ proc do_git_gui {} {
>>
>> # see note in do_gitk about unsetting these vars when
>> # running tools in a submodule
>> - unset env(GIT_DIR)
>> - unset env(GIT_WORK_TREE)
>> + unset_gitdir_vars
>>
>> set pwd [pwd]
>> cd $current_diff_path
>>
>> safe_exec_bg [concat $exe gui]
>>
>> - set env(GIT_DIR) $_gitdir
>> - set env(GIT_WORK_TREE) $_gitworktree
>> + set_gitdir_vars
>> cd $pwd
>>
>> set status_operation [$::main_status \
> After these changes, a 'global env' probably becomes stale and could be
> removed.
>
> -- Hannes
>
Will update to only ever set GIT_DIR, will still remove GIT_WORK_TREE on unset.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (2 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 03/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-15 11:00 ` Aina Boot
2026-05-15 15:59 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 05/11] git-gui: use --absolute-git-dir Mark Levedahl
` (8 subsequent siblings)
12 siblings, 2 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, 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.
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
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
+}
+
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
}
# Use object format as hash algorithm (either "sha1" or "sha256")
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
2026-05-14 14:33 ` [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc Mark Levedahl
@ 2026-05-15 11:00 ` Aina Boot
2026-05-15 13:33 ` Mark Levedahl
2026-05-15 15:59 ` Johannes Sixt
1 sibling, 1 reply; 96+ messages in thread
From: Aina Boot @ 2026-05-15 11:00 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Johannes Sixt, Shroom Moo, git
On 5/14/26 2:33 PM, Mark Levedahl wrote:
> 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
> +}
> +
>
Here inside the proc it create vars locally, "global..." is missing.
Aina
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
2026-05-15 11:00 ` Aina Boot
@ 2026-05-15 13:33 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-15 13:33 UTC (permalink / raw)
To: Aina Boot; +Cc: Johannes Sixt, Shroom Moo, git
On 5/15/26 7:00 AM, Aina Boot wrote:
> On 5/14/26 2:33 PM, Mark Levedahl wrote:
>> 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
>> +}
>> +
>>
> Here inside the proc it create vars locally, "global..." is missing.
>
> Aina
Yes, also, I missed copying the check on the return variable ::_gitdir. That proc should be:
proc pick_repo {} {
global _gitdir picked
unset_gitdir_vars
load_config 1
apply_config
choose_repository::pick
if {![file isdirectory $_gitdir]} {
exit 1
}
set _gitdir [git rev-parse --absolute-git-dir]
set picked 1
}
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
2026-05-14 14:33 ` [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc Mark Levedahl
2026-05-15 11:00 ` Aina Boot
@ 2026-05-15 15:59 ` Johannes Sixt
2026-05-16 14:29 ` Mark Levedahl
1 sibling, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-15 15:59 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
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/
> 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.
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.
> 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.
> }
>
> # Use object format as hash algorithm (either "sha1" or "sha256")
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc
2026-05-15 15:59 ` Johannes Sixt
@ 2026-05-16 14:29 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 14:29 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
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
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 05/11] git-gui: use --absolute-git-dir
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (3 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 04/11] git-gui: put choose_repository::pick in a proc Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-15 16:00 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 06/11] git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal Mark Levedahl
` (7 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git-gui uses git rev-parse --git-dir to get the pathname of the
discovered git repository. The returned value can be relative, and is
'.' if the current directory is the top of the repository directory
itself. git-gui has code to change '.' to [pwd] in this case so that
subsequent logic runs.
But, git rev-parse supports --absolute-git-dir from fac60b8925
("rev-parse: add option for absolute or relative path formatting",
2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
this more useful form is always available. Use --absolute-git-dir to
always get an absolute path, avoiding the need for other checks.
---
git-gui.sh | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 0b73c35..c2cf5f1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1156,7 +1156,7 @@ if {[catch {
&& [catch {
# beware that from the .git dir this sets _gitdir to .
# and _prefix to the empty string
- set _gitdir [git rev-parse --git-dir]
+ set _gitdir [git rev-parse --absolute-git-dir]
set _prefix [git rev-parse --show-prefix]
} err]} {
pick_repo
@@ -1173,18 +1173,12 @@ if {$hashalgorithm eq "sha1"} {
exit 1
}
-# we expand the _gitdir when it's just a single dot (i.e. when we're being
-# run from the .git dir itself) lest the routines to find the worktree
-# get confused
-if {$_gitdir eq "."} {
- set _gitdir [pwd]
-}
-
if {![file isdirectory $_gitdir]} {
catch {wm withdraw .}
error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
exit 1
}
+
# _gitdir exists, so try loading the config
load_config 0
apply_config
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 05/11] git-gui: use --absolute-git-dir
2026-05-14 14:33 ` [PATCH v1 05/11] git-gui: use --absolute-git-dir Mark Levedahl
@ 2026-05-15 16:00 ` Johannes Sixt
2026-05-16 14:33 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-15 16:00 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui uses git rev-parse --git-dir to get the pathname of the
> discovered git repository. The returned value can be relative, and is
> '.' if the current directory is the top of the repository directory
> itself. git-gui has code to change '.' to [pwd] in this case so that
> subsequent logic runs.
>
> But, git rev-parse supports --absolute-git-dir from fac60b8925
> ("rev-parse: add option for absolute or relative path formatting",
> 2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
> this more useful form is always available. Use --absolute-git-dir to
> always get an absolute path, avoiding the need for other checks.
Nice!
However, the patch is incomplete. We set _gitdir also from
lib/choose_repository.tcl. I think it would be best to swap this patch
with patch 4/11, remove the _gitdir setters from the picker
implementation, and call `rev-parse --absolute-git-dir` like you did in
4/11. This depends on that the picker sets the current directory to the
top-level of the working tree with the embeded .git directory.
BTW, missing sign-off.
> ---
> git-gui.sh | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 0b73c35..c2cf5f1 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1156,7 +1156,7 @@ if {[catch {
> && [catch {
> # beware that from the .git dir this sets _gitdir to .
> # and _prefix to the empty string
> - set _gitdir [git rev-parse --git-dir]
> + set _gitdir [git rev-parse --absolute-git-dir]
> set _prefix [git rev-parse --show-prefix]
> } err]} {
> pick_repo
> @@ -1173,18 +1173,12 @@ if {$hashalgorithm eq "sha1"} {
> exit 1
> }
>
> -# we expand the _gitdir when it's just a single dot (i.e. when we're being
> -# run from the .git dir itself) lest the routines to find the worktree
> -# get confused
> -if {$_gitdir eq "."} {
> - set _gitdir [pwd]
> -}
> -
> if {![file isdirectory $_gitdir]} {
> catch {wm withdraw .}
> error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
> exit 1
> }
> +
> # _gitdir exists, so try loading the config
> load_config 0
> apply_config
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 05/11] git-gui: use --absolute-git-dir
2026-05-15 16:00 ` Johannes Sixt
@ 2026-05-16 14:33 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 14:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/15/26 12:00 PM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui uses git rev-parse --git-dir to get the pathname of the
>> discovered git repository. The returned value can be relative, and is
>> '.' if the current directory is the top of the repository directory
>> itself. git-gui has code to change '.' to [pwd] in this case so that
>> subsequent logic runs.
>>
>> But, git rev-parse supports --absolute-git-dir from fac60b8925
>> ("rev-parse: add option for absolute or relative path formatting",
>> 2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
>> this more useful form is always available. Use --absolute-git-dir to
>> always get an absolute path, avoiding the need for other checks.
> Nice!
>
> However, the patch is incomplete. We set _gitdir also from
> lib/choose_repository.tcl. I think it would be best to swap this patch
> with patch 4/11, remove the _gitdir setters from the picker
> implementation, and call `rev-parse --absolute-git-dir` like you did in
> 4/11. This depends on that the picker sets the current directory to the
> top-level of the working tree with the embeded .git directory.
>
> BTW, missing sign-off.
I will change the interface to the picker so that success / failure is a returned value
rather than _gitdir being non-empty, then rework order and content of these patches.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 06/11] git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (4 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 05/11] git-gui: use --absolute-git-dir Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-14 14:33 ` [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
` (6 subsequent siblings)
12 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git accepts any combination of GIT_DIR and GIT_WORK_TREE to override the
normal repository and worktree discovery process. git-gui should accept
any such valid configuration, but overriding the discovery process means
the user has assured that the combination of current directory, GIT_DIR,
and GIT_WORK_TREE will lead to the correct repository and worktree. As
such, an error found during discovery where either or both of GIT_DIR
and GIT_WORK_TREE are set is a fatal error, no further exploration
should be tried.
Provide a common proc to support displaying an error message and exiting
if GIT_DIR or GIT_WORK_TREE are in the environment.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index c2cf5f1..2e2ddc0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1122,6 +1122,24 @@ unset argv0dir
##
## repository setup
+proc is_gitvars_error {err} {
+ set havevars 0
+ set GIT_DIR {}
+ set GIT_WORK_TREE {}
+ catch {set GIT_DIR $::env(GIT_DIR); set havevars 1}
+ catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE) ; set havevars 1}
+
+ if {$havevars} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Invalid configuration:"] \
+ "\n" "GIT_DIR: " $GIT_DIR \
+ "\n" "GIT_WORK_TREE: " $GIT_WORK_TREE \
+ "\n\n$err"]
+ return 1
+ }
+ return 0
+}
+
proc set_gitdir_vars {} {
global _gitdir _gitworktree env
if {$_gitdir ne {}} {
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (5 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 06/11] git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-15 16:06 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
` (5 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git-gui attempts to use env(GIT_DIR) directly as the git repository,
accepting GIT_DIR if it is a directory. Only if that fails is git
rev-parse used to discover the repository. But, this avoids all of
git-core's validity checking on a repository, thus possibly deferring an
error to a later step, possibly unexpected. Repository validation should
be part of initial setup so that later processing does not need error
trapping for configuration errors.
Let's just invoke rev-parse so all error checking is done. Stop here if
the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
behavior and show the repository picker.
Also, remove a later check on whether _gitdir is a directory: that code
cannot be reached without rev-parse having validating the repository.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 2e2ddc0..81789dd 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -374,6 +374,7 @@ set _gitdir {}
set _gitworktree {}
set _isbare {}
set _githtmldir {}
+set _prefix {}
set _reponame {}
set _shellpath {@@SHELL_PATH@@}
@@ -1167,19 +1168,18 @@ proc pick_repo {} {
set picked 1
}
+# find repository.
if {[catch {
- set _gitdir $env(GIT_DIR)
- set _prefix {}
- }]
- && [catch {
- # beware that from the .git dir this sets _gitdir to .
- # and _prefix to the empty string
- set _gitdir [git rev-parse --absolute-git-dir]
- set _prefix [git rev-parse --show-prefix]
- } err]} {
+ set _gitdir [git rev-parse --absolute-git-dir]
+} err]} {
+ if {[is_gitvars_error $err]} {
+ exit 1
+ } else {
pick_repo
+ }
}
+
# Use object format as hash algorithm (either "sha1" or "sha256")
set hashalgorithm [git rev-parse --show-object-format]
if {$hashalgorithm eq "sha1"} {
@@ -1191,12 +1191,6 @@ if {$hashalgorithm eq "sha1"} {
exit 1
}
-if {![file isdirectory $_gitdir]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
- exit 1
-}
-
# _gitdir exists, so try loading the config
load_config 0
apply_config
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository
2026-05-14 14:33 ` [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
@ 2026-05-15 16:06 ` Johannes Sixt
2026-05-16 14:38 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-15 16:06 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui attempts to use env(GIT_DIR) directly as the git repository,
> accepting GIT_DIR if it is a directory. Only if that fails is git
> rev-parse used to discover the repository. But, this avoids all of
> git-core's validity checking on a repository, thus possibly deferring an
> error to a later step, possibly unexpected. Repository validation should
> be part of initial setup so that later processing does not need error
> trapping for configuration errors.
OK. If the user gave us GIT_DIR with our without GIT_WORK_TREE, then
that combination better be workable.
>
> Let's just invoke rev-parse so all error checking is done. Stop here if
> the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
> behavior and show the repository picker.
OK. But the paragraph is confusing, because a big "If an error occurs"
is missing after the first sentence.
>
> Also, remove a later check on whether _gitdir is a directory: that code
> cannot be reached without rev-parse having validating the repository.
Good.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 2e2ddc0..81789dd 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -374,6 +374,7 @@ set _gitdir {}
> set _gitworktree {}
> set _isbare {}
> set _githtmldir {}
> +set _prefix {}
> set _reponame {}
> set _shellpath {@@SHELL_PATH@@}
>
> @@ -1167,19 +1168,18 @@ proc pick_repo {} {
> set picked 1
> }
>
> +# find repository.
> if {[catch {
> - set _gitdir $env(GIT_DIR)
> - set _prefix {}
> - }]
> - && [catch {
> - # beware that from the .git dir this sets _gitdir to .
> - # and _prefix to the empty string
> - set _gitdir [git rev-parse --absolute-git-dir]
> - set _prefix [git rev-parse --show-prefix]
> - } err]} {
> + set _gitdir [git rev-parse --absolute-git-dir]
Please do also set _prefix. It should fix the bug that the file chooser
uses an empty prefix after
cd lib
GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser master .
(this is an old bug.)
Please keep the additional indentation of the catch body.
> +} err]} {
> + if {[is_gitvars_error $err]} {
> + exit 1
> + } else {
> pick_repo
> + }
Treat the 'if' as an early exist without an else, and we don't need the
previously strange indentation of 'pick_repo'.
> }
>
> +
> # Use object format as hash algorithm (either "sha1" or "sha256")
> set hashalgorithm [git rev-parse --show-object-format]
> if {$hashalgorithm eq "sha1"} {
> @@ -1191,12 +1191,6 @@ if {$hashalgorithm eq "sha1"} {
> exit 1
> }
>
> -if {![file isdirectory $_gitdir]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
> - exit 1
> -}
> -
> # _gitdir exists, so try loading the config
> load_config 0
> apply_config
(Stopping the review here for today.)
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository
2026-05-15 16:06 ` Johannes Sixt
@ 2026-05-16 14:38 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 14:38 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/15/26 12:06 PM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui attempts to use env(GIT_DIR) directly as the git repository,
>> accepting GIT_DIR if it is a directory. Only if that fails is git
>> rev-parse used to discover the repository. But, this avoids all of
>> git-core's validity checking on a repository, thus possibly deferring an
>> error to a later step, possibly unexpected. Repository validation should
>> be part of initial setup so that later processing does not need error
>> trapping for configuration errors.
> OK. If the user gave us GIT_DIR with our without GIT_WORK_TREE, then
> that combination better be workable.
>
>> Let's just invoke rev-parse so all error checking is done. Stop here if
>> the user set GIT_DIR or GIT_WORK_TREE. Otherwise, continue the existing
>> behavior and show the repository picker.
> OK. But the paragraph is confusing, because a big "If an error occurs"
> is missing after the first sentence.
will fix.
>> Also, remove a later check on whether _gitdir is a directory: that code
>> cannot be reached without rev-parse having validating the repository.
> Good.
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
>> ---
>> git-gui.sh | 24 +++++++++---------------
>> 1 file changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index 2e2ddc0..81789dd 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -374,6 +374,7 @@ set _gitdir {}
>> set _gitworktree {}
>> set _isbare {}
>> set _githtmldir {}
>> +set _prefix {}
>> set _reponame {}
>> set _shellpath {@@SHELL_PATH@@}
>>
>> @@ -1167,19 +1168,18 @@ proc pick_repo {} {
>> set picked 1
>> }
>>
>> +# find repository.
>> if {[catch {
>> - set _gitdir $env(GIT_DIR)
>> - set _prefix {}
>> - }]
>> - && [catch {
>> - # beware that from the .git dir this sets _gitdir to .
>> - # and _prefix to the empty string
>> - set _gitdir [git rev-parse --absolute-git-dir]
>> - set _prefix [git rev-parse --show-prefix]
>> - } err]} {
>> + set _gitdir [git rev-parse --absolute-git-dir]
> Please do also set _prefix. It should fix the bug that the file chooser
> uses an empty prefix after
>
> cd lib
> GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser master .
>
> (this is an old bug.)
>
> Please keep the additional indentation of the catch body.
>
>> +} err]} {
>> + if {[is_gitvars_error $err]} {
>> + exit 1
>> + } else {
>> pick_repo
>> + }
> Treat the 'if' as an early exist without an else, and we don't need the
> previously strange indentation of 'pick_repo'.
>
>> }
>>
>> +
>> # Use object format as hash algorithm (either "sha1" or "sha256")
>> set hashalgorithm [git rev-parse --show-object-format]
>> if {$hashalgorithm eq "sha1"} {
>> @@ -1191,12 +1191,6 @@ if {$hashalgorithm eq "sha1"} {
>> exit 1
>> }
>>
>> -if {![file isdirectory $_gitdir]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
>> - exit 1
>> -}
>> -
>> # _gitdir exists, so try loading the config
>> load_config 0
>> apply_config
will fix all.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (6 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 07/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-16 8:12 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree Mark Levedahl
` (4 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git-gui includes proc is_bare, used in several places to make decisions
on whether a worktree exists, but also in discovery to tell if a
worktree can be supported.
But, is_bare is out of date with regard to multiple worktrees, safe
repository guards, and possibly other relevant features known to git
rev-parse. Also, is_bare caches its result on the first call, so is not
useful if a later step in the discovery process finds a worktree.
So, simplify is_bare to report whether git-gui has a worktree or is
working only from a repository.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 81789dd..a03eaa7 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -372,7 +372,6 @@ if {[tk windowingsystem] eq "aqua"} {
set _appname {Git Gui}
set _gitdir {}
set _gitworktree {}
-set _isbare {}
set _githtmldir {}
set _prefix {}
set _reponame {}
@@ -524,29 +523,7 @@ proc get_config {name} {
}
proc is_bare {} {
- global _isbare
- global _gitdir
- global _gitworktree
-
- if {$_isbare eq {}} {
- if {[catch {
- set _bare [git rev-parse --is-bare-repository]
- switch -- $_bare {
- true { set _isbare 1 }
- false { set _isbare 0}
- default { throw }
- }
- }]} {
- if {[is_config_true core.bare]
- || ($_gitworktree eq {}
- && [lindex [file split $_gitdir] end] ne {.git})} {
- set _isbare 1
- } else {
- set _isbare 0
- }
- }
- }
- return $_isbare
+ return [expr {$::_gitworktree eq {}}]
}
######################################################################
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known
2026-05-14 14:33 ` [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
@ 2026-05-16 8:12 ` Johannes Sixt
0 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-16 8:12 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui includes proc is_bare, used in several places to make decisions
> on whether a worktree exists, but also in discovery to tell if a
> worktree can be supported.
>
> But, is_bare is out of date with regard to multiple worktrees, safe
> repository guards, and possibly other relevant features known to git
> rev-parse. Also, is_bare caches its result on the first call, so is not
> useful if a later step in the discovery process finds a worktree.
>
> So, simplify is_bare to report whether git-gui has a worktree or is
> working only from a repository.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 25 +------------------------
> 1 file changed, 1 insertion(+), 24 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 81789dd..a03eaa7 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -372,7 +372,6 @@ if {[tk windowingsystem] eq "aqua"} {
> set _appname {Git Gui}
> set _gitdir {}
> set _gitworktree {}
> -set _isbare {}
> set _githtmldir {}
> set _prefix {}
> set _reponame {}
> @@ -524,29 +523,7 @@ proc get_config {name} {
> }
>
> proc is_bare {} {
> - global _isbare
> - global _gitdir
> - global _gitworktree
> -
> - if {$_isbare eq {}} {
> - if {[catch {
> - set _bare [git rev-parse --is-bare-repository]
> - switch -- $_bare {
> - true { set _isbare 1 }
> - false { set _isbare 0}
> - default { throw }
> - }
> - }]} {
> - if {[is_config_true core.bare]
> - || ($_gitworktree eq {}
> - && [lindex [file split $_gitdir] end] ne {.git})} {
> - set _isbare 1
> - } else {
> - set _isbare 0
> - }
> - }
> - }
> - return $_isbare
> + return [expr {$::_gitworktree eq {}}]
> }
>
> ######################################################################
Very nice!
IMO, regardless of which way we end up rewriting repository discovery,
the end result should be that we can use $_gitworktree like this to tell
whether we are in a bare repository or not.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (7 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 08/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-16 8:14 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 10/11] git-gui: improve worktree discovery Mark Levedahl
` (3 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
2010-01-23), has had the intent to allow starting from inside a
repository, then switching to the parent directory if that is a valid
worktree.
This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
--show-toplevel without a worktree an error", 2019-11-19) in git, but
breaking this git-gui feature was unintentional.
Add a proc to test if the parent of the git repository is a valid
worktree, and set that directory as the worktree if so. Use invocations
of git rev-parse to assure all validity and safety checks included in
git-core are executed.
---
git-gui.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index a03eaa7..e326401 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1100,6 +1100,23 @@ unset argv0dir
##
## repository setup
+proc is_parent_worktree {} {
+ # Directory 'parent' of a repository named 'parent/.git' might be the worktree
+ set ok 0
+ if {[file tail $::_gitdir] eq {.git}} {
+ set gitdir_parent [file join $::_gitdir {..}]
+ set expected_worktree [file normalize $gitdir_parent]
+ catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
+ if {[string compare $expected_worktree $git_worktree] == 0} {
+ set ::_prefix {}
+ set ::_gitworktree $git_worktree
+ cd $git_worktree
+ set ok 1
+ }
+ }
+ return $ok
+}
+
proc is_gitvars_error {err} {
set havevars 0
set GIT_DIR {}
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree
2026-05-14 14:33 ` [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree Mark Levedahl
@ 2026-05-16 8:14 ` Johannes Sixt
2026-05-16 14:48 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-16 8:14 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
> 2010-01-23), has had the intent to allow starting from inside a
> repository, then switching to the parent directory if that is a valid
> worktree.
>
> This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
> --show-toplevel without a worktree an error", 2019-11-19) in git, but
> breaking this git-gui feature was unintentional.
>
> Add a proc to test if the parent of the git repository is a valid
> worktree, and set that directory as the worktree if so. Use invocations
> of git rev-parse to assure all validity and safety checks included in
> git-core are executed.
BTW, missing sign-off.
> ---
> git-gui.sh | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index a03eaa7..e326401 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1100,6 +1100,23 @@ unset argv0dir
> ##
> ## repository setup
>
> +proc is_parent_worktree {} {
> + # Directory 'parent' of a repository named 'parent/.git' might be the worktree
> + set ok 0
> + if {[file tail $::_gitdir] eq {.git}} {
> + set gitdir_parent [file join $::_gitdir {..}]
> + set expected_worktree [file normalize $gitdir_parent]
We have [file dirname ...]. Is there a reason to avoid it?
> + catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
> + if {[string compare $expected_worktree $git_worktree] == 0} {
The purpose of this check should be explained in a comment. I think it is:
For a repository with the database in a directory named .git we assume
that the working tree is the directory containing .git. But
configuration may point to a different worktree. Then we do not want to
hold on to our assumption.
However, whether [git -C elsewhere ...] uses the same gitdir that we
have discovered so far cannot be told from this piece of code alone.
Therefore, I think it is wrong to extract this check into a function.
Also, I don't think we can use string comparison here. On Windows, the
command returns the Windows style path, but Tcl my operate with a POSIX
style path.
> + set ::_prefix {}
> + set ::_gitworktree $git_worktree
> + cd $git_worktree
So many side-effects in a function whose name suggests that it only does
some checks. Please, don't do that.
> + set ok 1
> + }
> + }
> + return $ok
> +}
> +
> proc is_gitvars_error {err} {
> set havevars 0
> set GIT_DIR {}
In general, I am not a fan of commits that add new functions, but no
call sites. Please squash this into 10/11. Ditto for is_gitvars_error in
06/11.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree
2026-05-16 8:14 ` Johannes Sixt
@ 2026-05-16 14:48 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 14:48 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/16/26 4:14 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
>> 2010-01-23), has had the intent to allow starting from inside a
>> repository, then switching to the parent directory if that is a valid
>> worktree.
>>
>> This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
>> --show-toplevel without a worktree an error", 2019-11-19) in git, but
>> breaking this git-gui feature was unintentional.
>>
>> Add a proc to test if the parent of the git repository is a valid
>> worktree, and set that directory as the worktree if so. Use invocations
>> of git rev-parse to assure all validity and safety checks included in
>> git-core are executed.
> BTW, missing sign-off.
>
>> ---
>> git-gui.sh | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index a03eaa7..e326401 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1100,6 +1100,23 @@ unset argv0dir
>> ##
>> ## repository setup
>>
>> +proc is_parent_worktree {} {
>> + # Directory 'parent' of a repository named 'parent/.git' might be the worktree
>> + set ok 0
>> + if {[file tail $::_gitdir] eq {.git}} {
>> + set gitdir_parent [file join $::_gitdir {..}]
>> + set expected_worktree [file normalize $gitdir_parent]
> We have [file dirname ...]. Is there a reason to avoid it?
>
>> + catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
>> + if {[string compare $expected_worktree $git_worktree] == 0} {
> The purpose of this check should be explained in a comment. I think it is:
>
> For a repository with the database in a directory named .git we assume
> that the working tree is the directory containing .git. But
> configuration may point to a different worktree. Then we do not want to
> hold on to our assumption.
>
> However, whether [git -C elsewhere ...] uses the same gitdir that we
> have discovered so far cannot be told from this piece of code alone.
> Therefore, I think it is wrong to extract this check into a function.
>
> Also, I don't think we can use string comparison here. On Windows, the
> command returns the Windows style path, but Tcl my operate with a POSIX
> style path.
As you have correctly inferred, am trying to unambiguously establish that git running in
the parent directory is using the child .git as the repository. I think this actually
requires two calls to git-revparse (--absolute-git-dir and --show-toplevel).
- the current git repo is valid to support a worktree. Will rework.
>> + set ::_prefix {}
>> + set ::_gitworktree $git_worktree
>> + cd $git_worktree
> So many side-effects in a function whose name suggests that it only does
> some checks. Please, don't do that.
>
>> + set ok 1
>> + }
>> + }
>> + return $ok
>> +}
>> +
>> proc is_gitvars_error {err} {
>> set havevars 0
>> set GIT_DIR {}
> In general, I am not a fan of commits that add new functions, but no
> call sites. Please squash this into 10/11. Ditto for is_gitvars_error in
> 06/11.
>
> -- Hannes
>
Next round should address all of your comments.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 10/11] git-gui: improve worktree discovery
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (8 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 09/11] git-gui: support using repository parent dir as a worktree Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-16 8:16 ` Johannes Sixt
2026-05-14 14:33 ` [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
` (2 subsequent siblings)
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
git gui's worktree discovery needs update based upon prior work in this
series. In the normal case, all information we need comes directly from
git rev-parse (--show-toplevel, and --show-prefix). Should this work, we
have a valid worktree and all git gui commands can run.
If not, we need to consider:
- if GIT_DIR or GIT_WORK_TREE are in the environment, just stop as we
the input configuration was wrong, the user must fix that.
- if we have a browser or blame subcommand, no worktree is needed so
git-gui can run without.
- using the git repository's parent is a valid worktree (if possible),
restoring prior behavior.
The current directory should be either the root of the worktree, if one
is found, or the top-level of the git repository.
Make it so. Also, make worktree discover directly follow repository
discovery, reducing the locations that might need error trapping to
catch configuration issues.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 56 ++++++++++++++++++++++--------------------------------
1 file changed, 23 insertions(+), 33 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index e326401..3a83dd5 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1173,6 +1173,28 @@ if {[catch {
}
}
+# find worktree, continue without if not required
+if {[catch {
+ set _gitworktree [git rev-parse --show-toplevel]
+ set _prefix [git rev-parse --show-prefix]
+ cd $_gitworktree
+} err]} {
+ if {[is_gitvars_error $err]} {
+ exit 1
+ }
+ set _gitworktree {}
+ set _prefix {}
+ if {[is_enabled bare]} {
+ cd $_gitdir
+ } elseif {![is_parent_worktree]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
+ exit 1
+ }
+}
+
+# repository and worktree config are complete, export them
+set_gitdir_vars
# Use object format as hash algorithm (either "sha1" or "sha256")
set hashalgorithm [git rev-parse --show-object-format]
@@ -1189,37 +1211,8 @@ if {$hashalgorithm eq "sha1"} {
load_config 0
apply_config
-set _gitworktree [git rev-parse --show-toplevel]
-if {$_prefix ne {}} {
- if {$_gitworktree eq {}} {
- regsub -all {[^/]+/} $_prefix ../ cdup
- } else {
- set cdup $_gitworktree
- }
- if {[catch {cd $cdup} err]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
- exit 1
- }
- set _gitworktree [pwd]
- unset cdup
-} elseif {![is_enabled bare]} {
- if {[is_bare]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
- exit 1
- }
- if {$_gitworktree eq {}} {
- set _gitworktree [file dirname $_gitdir]
- }
- if {[catch {cd $_gitworktree} err]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
- exit 1
- }
- set _gitworktree [pwd]
-}
+# Derive a human-readable repository name
set _reponame [file split [file normalize $_gitdir]]
if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end-1]
@@ -1227,9 +1220,6 @@ if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end]
}
-# Export the final paths
-set_gitdir_vars
-
######################################################################
##
## global init
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 10/11] git-gui: improve worktree discovery
2026-05-14 14:33 ` [PATCH v1 10/11] git-gui: improve worktree discovery Mark Levedahl
@ 2026-05-16 8:16 ` Johannes Sixt
2026-05-16 15:28 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-16 8:16 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git gui's worktree discovery needs update based upon prior work in this
> series. In the normal case, all information we need comes directly from
> git rev-parse (--show-toplevel, and --show-prefix). Should this work, we
> have a valid worktree and all git gui commands can run.
>
> If not, we need to consider:
> - if GIT_DIR or GIT_WORK_TREE are in the environment, just stop as we
> the input configuration was wrong, the user must fix that.
> - if we have a browser or blame subcommand, no worktree is needed so
> git-gui can run without.
> - using the git repository's parent is a valid worktree (if possible),
> restoring prior behavior.
>
> The current directory should be either the root of the worktree, if one
> is found, or the top-level of the git repository.
I disagree in the case where no working tree is found. Then there is no
point in changing the current directory.
>
> Make it so. Also, make worktree discover directly follow repository
> discovery, reducing the locations that might need error trapping to
> catch configuration issues.
Good!
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
BTW, please make the subject line more descriptive. The word "improve"
does not convey anything of importance.
> ---
> git-gui.sh | 56 ++++++++++++++++++++++--------------------------------
> 1 file changed, 23 insertions(+), 33 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index e326401..3a83dd5 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1173,6 +1173,28 @@ if {[catch {
> }
> }
>
> +# find worktree, continue without if not required
> +if {[catch {
> + set _gitworktree [git rev-parse --show-toplevel]
> + set _prefix [git rev-parse --show-prefix]
> + cd $_gitworktree
> +} err]} {
We have three commands, each with their own possible failure sources.
One of the outcomes of an error is that we proceed anyway. I think that
this is incorrect if the 'cd' fails: we must not proceed if it fails.
Therefore, we must handle its failure separately.
> + if {[is_gitvars_error $err]} {
> + exit 1
> + }
> + set _gitworktree {}
> + set _prefix {}
> + if {[is_enabled bare]} {
> + cd $_gitdir
Why change the directory here? If we run `git gui browser master dir` we
do not want to change the directory in an uncontrolled manner. The
argument parser will want to check for the existence of files, and then
we do not want to operate from a random directory.
Also, I think that the check must be for [is_bare] and not [is_enabled
bare].
> + } elseif {![is_parent_worktree]} {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
> + exit 1
> + }
> +}
> +
> +# repository and worktree config are complete, export them
> +set_gitdir_vars
>
> # Use object format as hash algorithm (either "sha1" or "sha256")
> set hashalgorithm [git rev-parse --show-object-format]
This moves code around. In particular, we see load_config and
apply_config in the context below, which now happens only after these
calls. How certain are we that these have no effect on the code that
runs now earlier?
> @@ -1189,37 +1211,8 @@ if {$hashalgorithm eq "sha1"} {
> load_config 0
> apply_config
>
> -set _gitworktree [git rev-parse --show-toplevel]
>
> -if {$_prefix ne {}} {
> - if {$_gitworktree eq {}} {
> - regsub -all {[^/]+/} $_prefix ../ cdup
> - } else {
> - set cdup $_gitworktree
> - }
> - if {[catch {cd $cdup} err]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
> - exit 1
> - }
> - set _gitworktree [pwd]
> - unset cdup
> -} elseif {![is_enabled bare]} {
> - if {[is_bare]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
> - exit 1
> - }
> - if {$_gitworktree eq {}} {
> - set _gitworktree [file dirname $_gitdir]
> - }
> - if {[catch {cd $_gitworktree} err]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
> - exit 1
> - }
> - set _gitworktree [pwd]
> -}
> +# Derive a human-readable repository name
> set _reponame [file split [file normalize $_gitdir]]
> if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end-1]
> @@ -1227,9 +1220,6 @@ if {[lindex $_reponame end] eq {.git}} {
> set _reponame [lindex $_reponame end]
> }
>
> -# Export the final paths
> -set_gitdir_vars
> -
> ######################################################################
> ##
> ## global init
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 10/11] git-gui: improve worktree discovery
2026-05-16 8:16 ` Johannes Sixt
@ 2026-05-16 15:28 ` Mark Levedahl
2026-05-19 8:16 ` Johannes Sixt
0 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 15:28 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/16/26 4:16 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> git gui's worktree discovery needs update based upon prior work in this
>> series. In the normal case, all information we need comes directly from
>> git rev-parse (--show-toplevel, and --show-prefix). Should this work, we
>> have a valid worktree and all git gui commands can run.
>>
>> If not, we need to consider:
>> - if GIT_DIR or GIT_WORK_TREE are in the environment, just stop as we
>> the input configuration was wrong, the user must fix that.
>> - if we have a browser or blame subcommand, no worktree is needed so
>> git-gui can run without.
>> - using the git repository's parent is a valid worktree (if possible),
>> restoring prior behavior.
>>
>> The current directory should be either the root of the worktree, if one
>> is found, or the top-level of the git repository.
> I disagree in the case where no working tree is found. Then there is no
> point in changing the current directory.
git-gui always changes to root of the worktree if that is found.
Failing to cd to the root of the repository when operating with no worktree opens the
possibility th
>
>> Make it so. Also, make worktree discover directly follow repository
>> discovery, reducing the locations that might need error trapping to
>> catch configuration issues.
> Good!
>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> BTW, please make the subject line more descriptive. The word "improve"
> does not convey anything of importance.
>
>> ---
>> git-gui.sh | 56 ++++++++++++++++++++++--------------------------------
>> 1 file changed, 23 insertions(+), 33 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index e326401..3a83dd5 100755
>> --- a/git-gui.sh
>> +++ b/git-gui.sh
>> @@ -1173,6 +1173,28 @@ if {[catch {
>> }
>> }
>>
>> +# find worktree, continue without if not required
>> +if {[catch {
>> + set _gitworktree [git rev-parse --show-toplevel]
>> + set _prefix [git rev-parse --show-prefix]
>> + cd $_gitworktree
>> +} err]} {
> We have three commands, each with their own possible failure sources.
> One of the outcomes of an error is that we proceed anyway. I think that
> this is incorrect if the 'cd' fails: we must not proceed if it fails.
> Therefore, we must handle its failure separately.
>
>> + if {[is_gitvars_error $err]} {
>> + exit 1
>> + }
>> + set _gitworktree {}
>> + set _prefix {}
>> + if {[is_enabled bare]} {
>> + cd $_gitdir
> Why change the directory here? If we run `git gui browser master dir` we
> do not want to change the directory in an uncontrolled manner. The
> argument parser will want to check for the existence of files, and then
> we do not want to operate from a random directory.
>
> Also, I think that the check must be for [is_bare] and not [is_enabled
> bare].
[is_enabled_bare] is correct. This code handles the case:
- neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
is currently true.
- the command given is browser or blame so a worktree is not needed. We can proceed.
The bigger question is whether to change directory at all: git-gui should never touch
files that are neither in the worktree nor in the repository. Leaving the current
directory as neither of those could be troublesome. I have no strong feeling here though,
will delete this.
>> + } elseif {![is_parent_worktree]} {
>> + catch {wm withdraw .}
>> + error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
>> + exit 1
>> + }
>> +}
>> +
>> +# repository and worktree config are complete, export them
>> +set_gitdir_vars
>>
>> # Use object format as hash algorithm (either "sha1" or "sha256")
>> set hashalgorithm [git rev-parse --show-object-format]
> This moves code around. In particular, we see load_config and
> apply_config in the context below, which now happens only after these
> calls. How certain are we that these have no effect on the code that
> runs now earlier?
We need to load the system and user global config before running the repository picker. We
(re-) load the full config including the repository after we have a repository. I think
this is correct: git-config explicitly lists worktree dependent includeif statements,
meaning the config can be worktree dependent, and we must not load the final config until
repository and worktree discovery are complete.
git rev-parse, etc., perform discovery and config file loading each time they are invoked,
those are unaffected by git-gui's internal config.
I will clarify this explicitly in the commit message.
>> @@ -1189,37 +1211,8 @@ if {$hashalgorithm eq "sha1"} {
>> load_config 0
>> apply_config
>>
>> -set _gitworktree [git rev-parse --show-toplevel]
>>
>> -if {$_prefix ne {}} {
>> - if {$_gitworktree eq {}} {
>> - regsub -all {[^/]+/} $_prefix ../ cdup
>> - } else {
>> - set cdup $_gitworktree
>> - }
>> - if {[catch {cd $cdup} err]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
>> - exit 1
>> - }
>> - set _gitworktree [pwd]
>> - unset cdup
>> -} elseif {![is_enabled bare]} {
>> - if {[is_bare]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
>> - exit 1
>> - }
>> - if {$_gitworktree eq {}} {
>> - set _gitworktree [file dirname $_gitdir]
>> - }
>> - if {[catch {cd $_gitworktree} err]} {
>> - catch {wm withdraw .}
>> - error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
>> - exit 1
>> - }
>> - set _gitworktree [pwd]
>> -}
>> +# Derive a human-readable repository name
>> set _reponame [file split [file normalize $_gitdir]]
>> if {[lindex $_reponame end] eq {.git}} {
>> set _reponame [lindex $_reponame end-1]
>> @@ -1227,9 +1220,6 @@ if {[lindex $_reponame end] eq {.git}} {
>> set _reponame [lindex $_reponame end]
>> }
>>
>> -# Export the final paths
>> -set_gitdir_vars
>> -
>> ######################################################################
>> ##
>> ## global init
> -- Hannes
>
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 10/11] git-gui: improve worktree discovery
2026-05-16 15:28 ` Mark Levedahl
@ 2026-05-19 8:16 ` Johannes Sixt
2026-05-19 19:00 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-19 8:16 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 16.05.26 um 17:28 schrieb Mark Levedahl:
> On 5/16/26 4:16 AM, Johannes Sixt wrote:
>> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>>> + if {[is_gitvars_error $err]} {
>>> + exit 1
>>> + }
>>> + set _gitworktree {}
>>> + set _prefix {}
>>> + if {[is_enabled bare]} {
>>> + cd $_gitdir
>> Why change the directory here? If we run `git gui browser master dir` we
>> do not want to change the directory in an uncontrolled manner. The
>> argument parser will want to check for the existence of files, and then
>> we do not want to operate from a random directory.
>>
>> Also, I think that the check must be for [is_bare] and not [is_enabled
>> bare].
>
> [is_enabled_bare] is correct. This code handles the case:
> - neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
> is currently true.
> - the command given is browser or blame so a worktree is not needed. We can proceed.
But in the case where the command is browser or blame, the argument
parser must later check for the existence of files, provided that a
worktree is present. But this conditional would change directory to
somewhere that is not a worktree at all even though a worktree is
available. So, I am still convinced that [is_bare] is correct.
> The bigger question is whether to change directory at all: git-gui should never touch
> files that are neither in the worktree nor in the repository. Leaving the current
> directory as neither of those could be troublesome. I have no strong feeling here though,
> will delete this.
OK. We need the conditional, but not change the directory.
>>> + } elseif {![is_parent_worktree]} {
>>> + catch {wm withdraw .}
>>> + error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
>>> + exit 1
>>> + }
>>> +}
>>> +
>>> +# repository and worktree config are complete, export them
>>> +set_gitdir_vars
>>>
>>> # Use object format as hash algorithm (either "sha1" or "sha256")
>>> set hashalgorithm [git rev-parse --show-object-format]
>> This moves code around. In particular, we see load_config and
>> apply_config in the context below, which now happens only after these
>> calls. How certain are we that these have no effect on the code that
>> runs now earlier?
> We need to load the system and user global config before running the repository picker. We
> (re-) load the full config including the repository after we have a repository. I think
> this is correct: git-config explicitly lists worktree dependent includeif statements,
> meaning the config can be worktree dependent, and we must not load the final config until
> repository and worktree discovery are complete.
Good point.
>
> git rev-parse, etc., perform discovery and config file loading each time they are invoked,
> those are unaffected by git-gui's internal config.
Also very true. Only calls to proc get_config would be affected by a
different setup, but there are none in the code that has swapped places
with working tree discovery. Only --show-object-format would be affected
by the discovered environment, and for that case it is more correct to
operate in the final setup.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 10/11] git-gui: improve worktree discovery
2026-05-19 8:16 ` Johannes Sixt
@ 2026-05-19 19:00 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-19 19:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/19/26 4:16 AM, Johannes Sixt wrote:
> Am 16.05.26 um 17:28 schrieb Mark Levedahl:
>> On 5/16/26 4:16 AM, Johannes Sixt wrote:
>>> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>>>> + if {[is_gitvars_error $err]} {
>>>> + exit 1
>>>> + }
>>>> + set _gitworktree {}
>>>> + set _prefix {}
>>>> + if {[is_enabled bare]} {
>>>> + cd $_gitdir
>>> Why change the directory here? If we run `git gui browser master dir` we
>>> do not want to change the directory in an uncontrolled manner. The
>>> argument parser will want to check for the existence of files, and then
>>> we do not want to operate from a random directory.
>>>
>>> Also, I think that the check must be for [is_bare] and not [is_enabled
>>> bare].
>> [is_enabled_bare] is correct. This code handles the case:
>> - neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
>> is currently true.
>> - the command given is browser or blame so a worktree is not needed. We can proceed.
> But in the case where the command is browser or blame, the argument
> parser must later check for the existence of files, provided that a
> worktree is present. But this conditional would change directory to
> somewhere that is not a worktree at all even though a worktree is
> available. So, I am still convinced that [is_bare] is correct.
I did change this. but...
I have reworked the blame/browser parser so it fully matches git blame parsing for the
single rev + path (or path rev) cases, all now do the same thing with or without a
worktree as they all work from git history, so having a worktree becomes almost moot (I
found issues with how git-gui handles --, it is dead wrong in some cases if the intent is
to match git-blame). What doesn't change is what blame displays based upon uncommitted or
staged changes in the worktree (if it does anything, but comments from 2007 suggest it
does, I haven't tested). I've done nothing to change that, only finding the args to pass
in to browser / blame. So, if a worktree exists, blame will still use the info there as it
does now.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (9 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 10/11] git-gui: improve worktree discovery Mark Levedahl
@ 2026-05-14 14:33 ` Mark Levedahl
2026-05-16 8:18 ` Johannes Sixt
2026-05-16 8:28 ` [PATCH v1 00/11] Improve git gui operation without a worktree Johannes Sixt
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
12 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-14 14:33 UTC (permalink / raw)
To: git; +Cc: egg_mushroomcow, j6t, bootaina702, Mark Levedahl
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.
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]
@@ -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
+ }
+}
+
# 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 \
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-14 14:33 ` [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
@ 2026-05-16 8:18 ` Johannes Sixt
2026-05-16 15:42 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-16 8:18 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> 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.
>
> 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.
> @@ -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?
> # 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
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-16 8:18 ` Johannes Sixt
@ 2026-05-16 15:42 ` Mark Levedahl
2026-05-19 8:21 ` Johannes Sixt
0 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-16 15:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/16/26 4:18 AM, Johannes Sixt wrote:
> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>> 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.
>
>> 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.
>
>> @@ -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?
I think this makes code less understandable and more complicated. As written, the user
selects the repo, and now the standard discovery runs except that the picker cannot be re-run.
I like the current structure better.
>> # 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
>
Repeating, I need to understand what "picked" really does in the gui. Will definitely
address your question.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-16 15:42 ` Mark Levedahl
@ 2026-05-19 8:21 ` Johannes Sixt
2026-05-19 18:45 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Johannes Sixt @ 2026-05-19 8:21 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 16.05.26 um 17:42 schrieb Mark Levedahl:
> 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?
I think I would be happier with the structure
if not subcommand pick
discover gitdir
if error
set subcommand pick
if subcommand pick
pick_repo
set subcommand gui
because this clarifies that pick_repo must erase all current traces of
GIT_DIR and GIT_WORK_TREE from the envionment and must complete with a
valid setup.
With the structure in the proposed patch
if subcommand pick
pick_repo
set subcommand gui
discover gitdir
if error
pick_repo
we still need the same operation of pick_repo, but after it runs due to
a pick command, we go into "discover gitdir" mode in an already modified
environment, something that does not happen if pick_repo runs due to the
error in the gitdir discovery.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-19 8:21 ` Johannes Sixt
@ 2026-05-19 18:45 ` Mark Levedahl
2026-05-19 21:15 ` Johannes Sixt
0 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-19 18:45 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
On 5/19/26 4:21 AM, Johannes Sixt wrote:
> Am 16.05.26 um 17:42
>
>
> I think I would be happier with the structure
>
> if not subcommand pick
> discover gitdir
> if error
> set subcommand pick
>
> if subcommand pick
> pick_repo
> set subcommand gui
>
> because this clarifies that pick_repo must erase all current traces of
> GIT_DIR and GIT_WORK_TREE from the envionment and must complete with a
> valid setup.
>
> With the structure in the proposed patch
>
> if subcommand pick
> pick_repo
> set subcommand gui
>
> discover gitdir
> if error
> pick_repo
>
> we still need the same operation of pick_repo, but after it runs due to
> a pick command, we go into "discover gitdir" mode in an already modified
> environment, something that does not happen if pick_repo runs due to the
> error in the gitdir discovery.
>
> -- Hannes
>
What I have now is
if (enabled gitdir discovery) {
discover gitdir
maybe an error occurs and gitdir remains {}
}
if (enabled pick && gitdir eq {}) {
unset GIT_DIR .. (just to be friendly, could throw an error instead...)
pick
discover gitdir to VALIDATE pick gave us a good thing
}
if no gitdir {
error No Repository
}
then on to worktree discovery (which also validates what pick returns as pick may not have
done so).
So, we can independently enable normal discovery or pick, and with both enabled pick can
be used to recover from an error in normal discovery. Either way, there is only one block
of code running pick, it is not a separate proc invoked multiple places. I hope this
scratches your itch.
Still scrubbing things, should send out in a day or two.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-19 18:45 ` Mark Levedahl
@ 2026-05-19 21:15 ` Johannes Sixt
0 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-19 21:15 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 19.05.26 um 20:45 schrieb Mark Levedahl:
> What I have now is
>
> if (enabled gitdir discovery) {
> discover gitdir
> maybe an error occurs and gitdir remains {}
> }
>
> if (enabled pick && gitdir eq {}) {
> unset GIT_DIR .. (just to be friendly, could throw an error instead...)
> pick
> discover gitdir to VALIDATE pick gave us a good thing
> }
>
> if no gitdir {
> error No Repository
> }
>
> then on to worktree discovery (which also validates what pick returns as pick may not have
> done so).
Sounds good.
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH v1 00/11] Improve git gui operation without a worktree
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (10 preceding siblings ...)
2026-05-14 14:33 ` [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
@ 2026-05-16 8:28 ` Johannes Sixt
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
12 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-16 8:28 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 14.05.26 um 16:33 schrieb Mark Levedahl:
> git gui has a number of inter-related problems that result in problems
> during startup from anything but a checked out worktree pointing at a
> valid git repository. Some of the symptoms are:
> - blame / browser subcommands, and launching gitk, are intended to be
> useful without a worktree, but fail to work.
> - unlike git, git-gui is supposed to use the parent directory as a
> worktree if started from the .git subdirectory in the very common
> single worktree + embedded git repository format. This does not
> work.
> - git-gui includes a repository picker allowing a user select a worktree
> from a list and/or start a new repo+worktree: this dialog appears at
> unexpected times, masking useful error feedback on configuration
> problems.
>
> This patch series addresses the above issues, substantially rewriting
> the blame / browser command line process, the initial repository and
> worktree discovery processes, and using git rev-parse when possible to
> handle repository / worktree discovery including any specification of
> GIT_DIR or GIT_WORK_TREE to reduce the future likelihood of conflict
> with command line git. This also allows explicit user control to avoid
> the repository picker masking a configuration error.
OK. Overall, this goes in the right direction. There are still open
questions and potential problems with this implementation. We also
disagree in a few details; see my comments on the patches.
>
> Note: I question why git-gui ever exports GIT_WORK_TREE. If it is not
> empty, that is the current directory when startup is complete and any
> git command will use the current directory as the worktree.
I fully agree with this.
> If empty,
> there is no worktree and the current directory should be (and after this
> series, is) at the toplevel of the gitdir: again, there is nothing to
> communicate to another process.
Here I disagree. We should not need to change directory if no working
tree was found.
> If a process being launched needs a
> different worktree, that should be the startup directory given to the
> process without changing git-gui's current directory.
I haven't thought this through, but this sounds very reasonable.
>
> Mark Levedahl (11):
> git-gui: allow specifying path '.' to the browser
> git-gui: refactor browser / blame argument parsing
> git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
> git-gui: put choose_repository::pick in a proc
> git-gui: use --absolute-git-dir
> git gui: GIT_DIR / GIT_WORK_TREE make any discovery error fatal
> git-gui: use rev-parse exclusively to find a repository
> git-gui: simplify [is_bare] to report if a worktree is known
> git-gui: support using repository parent dir as a worktree
> git-gui: improve worktree discovery
> git-gui: add gui and pick as explicit subcommands
>
> git-gui.sh | 276 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 135 insertions(+), 141 deletions(-)
>
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread* [PATCH v2 00/11] Improve git gui operation without a worktree
2026-05-14 14:33 ` [PATCH v1 00/11] Improve git gui operation without a worktree Mark Levedahl
` (11 preceding siblings ...)
2026-05-16 8:28 ` [PATCH v1 00/11] Improve git gui operation without a worktree Johannes Sixt
@ 2026-05-20 20:23 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
` (10 more replies)
12 siblings, 11 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:23 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git gui has a number of inter-related problems that result in problems
during startup from anything but a checked out worktree pointing at a
valid git repository. Some of the symptoms are:
- blame / browser subcommands, and launching gitk, are intended to be
useful without a worktree, but fail to work.
- unlike git, git-gui is supposed to use the parent directory as a
worktree if started from the .git subdirectory in the very common
single worktree + embedded git repository format. This does not
work.
- git-gui includes a repository picker allowing a user to select a
worktree from a list and/or start a new repo+worktree: this dialog can
appear at unexpected times, masking useful error feedback on
configuration problems.
This patch series addresses the above issues, substantially rewriting
the initial repository/worktree process to rely upon git rev-parse so
that git's knowledge of access rules, repository configuration, and use
of GIT_DIR / GIT_WORK_TREE (or git --gitdir / --work-tree) is used
throughout, replacing code largely based upon what git did in 2008. This
also means that git gui will naturally gain any new rules implmented in
git-core.
With this, git-gui only exports GIT_WORK_TREE when non-empty.
GIT_WORK_TREE is needed, and must be exported, if the user is overriding
core.worktree in the git repository. But, GIT_WORK_TREE cannot be used
to specify the lack of a worktree, so exporting an empty GIT_WORK_TREE
is one of the problems fixed by this series.
v2 of this series is a very substantial rewrite driven by j6t's review,
with patches reoranized and squashed, interfaces to the repository
chooser changed, a different code structure to allow user control of the
repository picker, a different approach to fixing the command line
parser for blame / browser, and other more minor changes. Patches
for fixing blame / browser are now after all discovery refactoring as
they cannot be tested without some of those fixes.
Many subtle things are fixed beyond the list at the top, including
better compatibility with git blame and repeatable browser / blame
operation for specific revs not in the worktree, regardless of the
worktree state. j6t indicated that in the git-gui project, the following
fails in the current release:
cd lib
GIT_DIR=$PWD/../.git GIT_WORK_TREE=$PWD/.. ../git-gui.sh browser origin/master .
This is due to a _prefix issue, and is fixed as of the patch
git-gui: use git rev-parse for worktree discovery
Mark Levedahl (11):
git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
git-gui: return status from choose_repository::pick
git-gui: use --absolute-git-dir
git-gui: use rev-parse exclusively to find a repository
git-gui: simplify [is_bare] to report if a worktree is known
git-gui: use git rev-parse for worktree discovery
git-gui: try harder to find worktree from gitdir
git-gui: use HEAD as current branch when detached (bug fix)
git-gui: allow specifying path '.' to the browser
git-gui: adapt blame/browser parsing for bare operation
git-gui: add gui and pick as explicit subcommands
git-gui.sh | 412 +++++++++++++++++++++++---------------
lib/choose_repository.tcl | 21 +-
2 files changed, 257 insertions(+), 176 deletions(-)
Interdiff against v1:
diff --git a/git-gui.sh b/git-gui.sh
index c56aeeff88..299c1a0292 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -648,6 +648,9 @@ proc load_current_branch {} {
set current_branch [git branch --show-current]
set is_detached [expr [string length $current_branch] == 0]
+ if {$is_detached} {
+ set current_branch {HEAD}
+ }
}
auto_load tk_optionMenu
@@ -1021,7 +1024,8 @@ proc load_config {include_global} {
##
## feature option selection
-set run_picker_on_error 1
+enable_option picker
+enable_option gitdir_discovery
if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
unset _junk
} else {
@@ -1031,9 +1035,11 @@ 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]
+ if {$subcommand eq {gui}} {
+ disable_option picker
+ }
}
enable_option multicommit
@@ -1049,7 +1055,7 @@ blame {
disable_option multicommit
disable_option branch
disable_option transport
- set run_picker_on_error 0
+ disable_option picker
}
citool {
enable_option singlecommit
@@ -1058,7 +1064,7 @@ citool {
disable_option multicommit
disable_option branch
disable_option transport
- set run_picker_on_error 0
+ disable_option picker
while {[llength $argv] > 0} {
set a [lindex $argv 0]
@@ -1081,6 +1087,9 @@ citool {
set argv [lrange $argv 1 end]
}
}
+pick {
+ disable_option gitdir_discovery
+}
}
######################################################################
@@ -1104,21 +1113,39 @@ unset argv0dir
##
## repository setup
-proc is_parent_worktree {} {
- # Directory 'parent' of a repository named 'parent/.git' might be the worktree
- set ok 0
+proc find_worktree_from_gitdir {} {
+ # Directory 'parent' of a repository named 'parent/.git' might be the worktree.
+ # Assure parent is a worktree and using the git repository already discovered.
+ # Also, handle case of being in a worktree's gitdir, where file "gitdir" points to
+ # gitlink file .git in the real worktree.
+ set worktree {}
if {[file tail $::_gitdir] eq {.git}} {
- set gitdir_parent [file join $::_gitdir {..}]
- set expected_worktree [file normalize $gitdir_parent]
- catch {set git_worktree [git -C $gitdir_parent rev-parse --show-toplevel]}
- if {[string compare $expected_worktree $git_worktree] == 0} {
- set ::_prefix {}
- set ::_gitworktree $git_worktree
- cd $git_worktree
- set ok 1
+ if {[catch {
+ set gitdir_parent [file dirname $::_gitdir]
+ set worktree [git -C $gitdir_parent rev-parse --show-toplevel]
+ set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
+ if {$::_gitdir ne $parent_gitdir} {
+ set worktree {}
+ }
+ }]} {
+ set worktree {}
+ }
+ } elseif [file exists {gitdir}] {
+ if {[catch {
+ set fd_gitdir [open {gitdir} {r}]
+ set gitlink_parent [file dirname [read $fd_gitdir]]
+ catch {close $fd_gitdir}
+ set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
+ set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
+ if {$::_gitdir ne $parent_gitdir} {
+ set worktree {}
+ }
+ }]} {
+ catch {close $fd_gitdir}
+ set worktree {}
}
}
- return $ok
+ return $worktree
}
proc is_gitvars_error {err} {
@@ -1155,62 +1182,76 @@ proc unset_gitdir_vars {} {
catch {unset env(GIT_WORK_TREE)}
}
+# find repository.
+set _gitdir {}
+if {[is_enabled gitdir_discovery]} {
+ if {[catch {
+ set _gitdir [git rev-parse --absolute-git-dir]
+ } err]} {
+ if {[is_gitvars_error $err]} {
+ exit 1
+ }
+ set _gitdir {}
+ }
+}
+
set picked 0
-proc pick_repo {} {
+if {$_gitdir eq {} && [is_enabled picker]} {
unset_gitdir_vars
load_config 1
apply_config
- choose_repository::pick
- set _gitdir [git rev-parse --absolute-git-dir]
- set _prefix {}
+ if {![choose_repository::pick]} {
+ exit 1
+ }
+ if {[catch {
+ set _gitdir [git rev-parse --absolute-git-dir]
+ } err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
+ }
set picked 1
}
-# run repository picker if explicitly requested
-switch -- $subcommand {
- pick {
- pick_repo
- set subcommand gui
- set run_picker_on_error 0
- }
-}
-
-# find repository.
-if {[catch {
- set _gitdir [git rev-parse --absolute-git-dir]
-} err]} {
- if {[is_gitvars_error $err]} {
- exit 1
- }
- if {$run_picker_on_error} {
- pick_repo
- } else {
- catch {wm withdraw .}
- error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
- exit 1
- }
+if {$_gitdir eq {}} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
+ exit 1
}
# find worktree, continue without if not required
if {[catch {
set _gitworktree [git rev-parse --show-toplevel]
set _prefix [git rev-parse --show-prefix]
- cd $_gitworktree
} err]} {
if {[is_gitvars_error $err]} {
exit 1
}
set _gitworktree {}
set _prefix {}
- if {[is_enabled bare]} {
- cd $_gitdir
- } elseif {![is_parent_worktree]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
- exit 1
+}
+
+if {[is_bare]} {
+ # Maybe we are in an embedded or worktree specific gitdir
+ if {[set _gitworktree [find_worktree_from_gitdir]] ne {}} {
+ set _prefix {}
}
}
+if {![is_bare]} {
+ if {[catch {
+ cd $_gitworktree
+ } err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot change to discovered worktree: "] \
+ "$_gitworktree" "\n\n$err"]
+ exit 1;
+ }
+} elseif {![is_enabled bare]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
+ exit 1
+}
+
# repository and worktree config are complete, export them
set_gitdir_vars
@@ -1229,8 +1270,6 @@ if {$hashalgorithm eq "sha1"} {
load_config 0
apply_config
-
-# Derive a human-readable repository name
set _reponame [file split [file normalize $_gitdir]]
if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end-1]
@@ -2035,7 +2074,7 @@ proc incr_font_size {font {amt 1}} {
proc do_gitk {revs {is_submodule false}} {
global current_diff_path file_states current_diff_side ui_index
- global _gitdir _gitworktree
+ global _gitworktree
# -- Always start gitk through whatever we were loaded with. This
# lets us bypass using shell process on Windows systems.
@@ -2045,8 +2084,6 @@ proc do_gitk {revs {is_submodule false}} {
if {$exe eq {}} {
error_popup [mc "Couldn't find gitk in PATH"]
} else {
- global env
-
set pwd [pwd]
if {!$is_submodule} {
@@ -2105,9 +2142,6 @@ proc do_git_gui {} {
if {$exe eq {}} {
error_popup [mc "Couldn't find git gui in PATH"]
} else {
- global env
- global _gitdir _gitworktree
-
# see note in do_gitk about unsetting these vars when
# running tools in a submodule
unset_gitdir_vars
@@ -2992,77 +3026,135 @@ proc normalize_relpath {path} {
if {$elements ne {}} {
return [eval file join $elements]
} else {
- return {}
+ return {./}
}
}
+proc find_path_type {head path} {
+ if {$path eq {./}} {
+ # the root-tree exists in every rev, ls-tree gives data on the contents,
+ # not the type of tree itself. So, if the rev exists, return {tree}
+ if {[catch {set objtype [git ls-tree $head]}]} {
+ set objtype {}
+ } else {
+ set objtype {tree}
+ }
+ } else {
+ # test that the path exists in head, ls-tree gives info on the path only
+ if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
+ set objtype {}
+ }
+ }
+ return $objtype
+}
+
# -- Not a normal commit type invocation? Do that instead!
#
switch -- $subcommand {
browser -
blame {
if {$subcommand eq "blame"} {
- set subcommand_args {[--line=<num>] rev? path}
+ set subcommand_args {[--line=<num>] <[rev] [--] filename | [--] filename rev>}
+ set required_objtype blob
} else {
- set subcommand_args {rev? path}
+ set subcommand_args {<[rev] [--] directory | [--] directory rev>}
+ set required_objtype tree
}
- if {$argv eq {}} usage
- set head {}
- set path {}
- set jump_spec {}
+ set maxargs [llength $subcommand_args]
set nargs [llength $argv]
- if {$nargs < 1} {
- usage
- }
- set argn 0
- foreach a $argv {
- set argn [expr {$argn + 1}]
+ if {$nargs < 1 || $nargs > $maxargs} usage
+ set head {}
+ set althead {}
+ set path {}
+ set altpath {}
+ set canswap 1
+ set jump_spec {}
- if {$argn < $nargs} {
- # revision or line number
- if {[regexp {^--line=(\d+)$} $a a lnum]} {
- set jump_spec [list $lnum]
+ # assume: [--line=num] [head] [--] path as the possible arguments, in order.
+ # head and path may need a swap later.
+ for {set iarg 0} {$iarg < $nargs} {incr iarg} {
+ set arg [lindex $argv $iarg]
+ if {$arg eq {--}} {
+ # next arg is the path, prevent or FORCE swap?
+ if {$iarg == $nargs - 2} {
+ set canswap 0
+ } elseif {$iarg == $nargs - 3} {
+ set canswap 2
} else {
- set head $a
+ usage
}
+ } elseif {[regexp {^--line=(\d+)$} $arg arg lnum]} {
+ # --line can only be the first arg
+ if {$iarg != 0 || $maxargs < 4} usage
+ set jump_spec [list $lnum]
+ } elseif {$iarg == $nargs - 1} {
+ # assume final argument is path
+ set path [normalize_relpath [file join $_prefix $arg]]
+ set althead $arg
+ } elseif {$head eq {}} {
+ # assume the other argument is head
+ set head $arg
+ set altpath [normalize_relpath [file join $_prefix $arg]]
} else {
- set path [normalize_relpath $a]
+ usage
}
}
+ # no swapping allowed if head not given, use current branch (HEAD)
if {$head eq {}} {
load_current_branch
set head $current_branch
- } else {
- if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
- if {[catch {
- set head [git rev-parse --verify $head]
- } err]} {
- if {[tk windowingsystem] eq "win32"} {
- tk_messageBox -icon error -title [mc Error] -message $err
- } else {
- puts stderr $err
- }
- exit 1
- }
+ set canswap 0
+ }
+
+ # -- before "rev" arg means we got -- path head
+ if {$canswap == 2} {
+ set head $althead
+ set path $altpath
+ set canswap 0
+ }
+
+ set objtype [find_path_type $head $path]
+ if {$objtype eq {} && $canswap} {
+ set objtype [find_path_type $althead $altpath]
+ if {$objtype ne {}} {
+ set head $althead
+ set path $altpath
}
- set current_branch $head
+ }
+ set current_branch $head
+
+ # check that path exists in head, and objtype matches need
+ if {$objtype ne $required_objtype} {
+ switch -- $required_objtype {
+ tree {set err [strcat \
+ [mc "'%s' is not a directory in rev '%s'" $path $head]]}
+ blob {set err [strcat \
+ [mc "'%s' is not a filename in rev '%s'" $path $head]]}
+ }
+ if {[tk windowingsystem] eq "win32"} {
+ catch {wm withdraw .}
+ error_popup $err
+ } else {
+ puts stderr $err
+ }
+ exit 1
}
wm deiconify .
switch -- $subcommand {
browser {
- if {$jump_spec ne {}} usage
browser::new $head $path
}
- blame {
+ blame {
blame::new $head $path $jump_spec
}
}
return
}
citool -
-gui {
+gui -
+pick {
if {[llength $argv] != 0} {
usage
}
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 7e1462a20c..4b06afee93 100644
--- 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
}
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-22 8:06 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 02/11] git-gui: return status from choose_repository::pick Mark Levedahl
` (9 subsequent siblings)
10 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git-gui unconditionally exports _gitdir as GIT_DIR, and _gitworktree as
GIT_WORK_TREE, to the environment, and furthermore unconditionally
unsets these environment variables in many
git gui must have a repository, so _gitdir can never be empty and its
export is always valid if repository discovery completes successfully.
git gui might not find a worktree, so _gitworktree can be empty. While
having no worktree is valid for blame/browser subcommands, exporting
GIT_WORK_TREE=<empty> is not valid. Rather, an empty GIT_WORK_TREE
raises errors in git builtins, for instance 'git branch --show-current'
as used by git, and causes breakage. This is one cause of git blame /
git browser not working without a worktree.
A user may set GIT_DIR and/or GIT_WORK_TREE to override git's normal
discovery rules, including repository configuration of core.worktree
and/or worktree specific gitdirs. It is always safe to export the
absolute pathnames of the discovered values, even though they may not be
needed. However, the gitdir might not be found from the worktree without
GIT_DIR being set. Furthermore, the worktree defined by the discovered
gitdir might be overridden by GIT_WORK_TREE set before git-gui started.
So, it is also sometimes necessary that one or both of these variables
is set.
So, let's provide two procs, one to unset GIT_DIR / GIT_WORK_TREE if
they are set, one to set GIT_DIR and, if not empty, GIT_WORK_TREE, so
all call sites do the same thing, and problems with _gitworktree == {}
are avoided.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 23fe76e498..4ba25da7b6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1122,6 +1122,22 @@ unset argv0dir
##
## repository setup
+proc set_gitdir_vars {} {
+ global _gitdir _gitworktree env
+ if {$_gitdir ne {}} {
+ set env(GIT_DIR) $_gitdir
+ }
+ if {$_gitworktree ne {}} {
+ set env(GIT_WORK_TREE) $_gitworktree
+ }
+}
+
+proc unset_gitdir_vars {} {
+ global env
+ catch {unset env(GIT_DIR)}
+ catch {unset env(GIT_WORK_TREE)}
+}
+
set picked 0
if {[catch {
set _gitdir $env(GIT_DIR)
@@ -1207,8 +1223,8 @@ if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end]
}
-set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
+# Export the final paths
+set_gitdir_vars
######################################################################
##
@@ -2007,7 +2023,7 @@ proc incr_font_size {font {amt 1}} {
proc do_gitk {revs {is_submodule false}} {
global current_diff_path file_states current_diff_side ui_index
- global _gitdir _gitworktree
+ global _gitworktree
# -- Always start gitk through whatever we were loaded with. This
# lets us bypass using shell process on Windows systems.
@@ -2017,8 +2033,6 @@ proc do_gitk {revs {is_submodule false}} {
if {$exe eq {}} {
error_popup [mc "Couldn't find gitk in PATH"]
} else {
- global env
-
set pwd [pwd]
if {!$is_submodule} {
@@ -2050,13 +2064,11 @@ proc do_gitk {revs {is_submodule false}} {
# TODO we could make life easier (start up faster?) for gitk
# by setting these to the appropriate values to allow gitk
# to skip the heuristics to find their proper value
- unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
+ unset_gitdir_vars
}
safe_exec_bg [concat $cmd $revs "--" "--"]
- set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
+ set_gitdir_vars
cd $pwd
if {[info exists main_status]} {
@@ -2079,21 +2091,16 @@ proc do_git_gui {} {
if {$exe eq {}} {
error_popup [mc "Couldn't find git gui in PATH"]
} else {
- global env
- global _gitdir _gitworktree
-
# see note in do_gitk about unsetting these vars when
# running tools in a submodule
- unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
+ unset_gitdir_vars
set pwd [pwd]
cd $current_diff_path
safe_exec_bg [concat $exe gui]
- set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
+ set_gitdir_vars
cd $pwd
set status_operation [$::main_status \
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
2026-05-20 20:24 ` [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
@ 2026-05-22 8:06 ` Johannes Sixt
0 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-22 8:06 UTC (permalink / raw)
To: Mark Levedahl, git; +Cc: egg_mushroomcow, bootaina702
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git-gui unconditionally exports _gitdir as GIT_DIR, and _gitworktree as
> GIT_WORK_TREE, to the environment, and furthermore unconditionally
> unsets these environment variables in many
"many cases"?
>
> git gui must have a repository, so _gitdir can never be empty and its
> export is always valid if repository discovery completes successfully.
_gitdir cannot be empty, so we should be able to drop the conditionals
around 'set env(GIT_DIR) $_gitdir'.
>
> git gui might not find a worktree, so _gitworktree can be empty. While
> having no worktree is valid for blame/browser subcommands, exporting
> GIT_WORK_TREE=<empty> is not valid. Rather, an empty GIT_WORK_TREE
> raises errors in git builtins, for instance 'git branch --show-current'
> as used by git, and causes breakage. This is one cause of git blame /
> git browser not working without a worktree.
>
> A user may set GIT_DIR and/or GIT_WORK_TREE to override git's normal
> discovery rules, including repository configuration of core.worktree
> and/or worktree specific gitdirs. It is always safe to export the
> absolute pathnames of the discovered values, even though they may not be
> needed. However, the gitdir might not be found from the worktree without
> GIT_DIR being set. Furthermore, the worktree defined by the discovered
> gitdir might be overridden by GIT_WORK_TREE set before git-gui started.
> So, it is also sometimes necessary that one or both of these variables
> is set.
While all you say here is true, the actual reason for the dance is more
like the simpler: provide a clean slate for the new process and return
to the old state after it has been started.
>
> So, let's provide two procs, one to unset GIT_DIR / GIT_WORK_TREE if
> they are set, one to set GIT_DIR and, if not empty, GIT_WORK_TREE, so
> all call sites do the same thing, and problems with _gitworktree == {}
> are avoided.
That being said, I propose the two patches below (pasted here for
review), after which we do not need these functions anymore IMHO
because the call sites are one-liners around GIT_DIR anyway.
The commits are available here:
git fetch https://github.com/j6t/git-gui.git js/unset-git-work-tree
https://github.com/j6t/git-gui/tree/js/unset-git-work-tree
------ 8< ------
Subject: [PATCH 1/2] git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk
In the procedure that invokes Gitk, we have a 'cd $_gitworktree'. Such
a change of the current directory is not necessary, because
- if we have a working tree, then the startup routine has already
changed the current directory to the root of the working tree, which
*is* $_gitworktree; or
- if we are in a bare repository, then there is no point in changing
the current directory anywhere. (And $_gitworktree is empty.)
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
git-gui.sh | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 23fe76e498bd..8d2b02b13fa0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2021,11 +2021,7 @@ proc do_gitk {revs {is_submodule false}} {
set pwd [pwd]
- if {!$is_submodule} {
- if {![is_bare]} {
- cd $_gitworktree
- }
- } else {
+ if {$is_submodule} {
cd $current_diff_path
if {$revs eq {--}} {
set s $file_states($current_diff_path)
--
2.54.0.215.g4fe990ec16
------ 8< ------
Subject: [PATCH 2/2] git-gui: operate git commands without GIT_WORK_TREE
The manual page of the git command states about the --git-dir option:
Specifying the location of the ".git" directory using this option
(or GIT_DIR environment variable) turns off the repository
discovery [...], and tells Git that you are at the top level of
the working tree.
Use this to our advantage:
- Set GIT_DIR in the environment to the value that was discovered, so
that the invoked git commands operate on the same repository
database that Git GUI uses even after it changes the working
directory.
- After changing the working directory to the top level of the working
tree, ensure that GIT_WORK_TREE is not set, because, as per
documentation, all git invocations from then on will assume that the
current working directory is also the top level working tree.
- Remove the now obsolete GIT_WORK_TREE dance when subordinate Gitk or
Git GUI are invoked for a submodule.
Do keep the state of GIT_WORK_TREE if we are in a bare repository,
because Git GUI is not interested in the worktree at all, as no commit
mode is possible in a bare repository.
This avoids cases where an empty GIT_WORK_TREE was exported into the
environment, most notably by a call of `git gui blame HEAD file` in a
bare repository. (Although, this particular error is currently masked
by an earlier failure in `rev-parse --show-toplevel`, which requires a
working tree.)
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
git-gui.sh | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 8d2b02b13fa0..3819f8be2211 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1183,6 +1183,7 @@ if {$_prefix ne {}} {
exit 1
}
set _gitworktree [pwd]
+ catch {unset env(GIT_WORK_TREE)}
unset cdup
} elseif {![is_enabled bare]} {
if {[is_bare]} {
@@ -1199,6 +1200,7 @@ if {$_prefix ne {}} {
exit 1
}
set _gitworktree [pwd]
+ catch {unset env(GIT_WORK_TREE)}
}
set _reponame [file split [file normalize $_gitdir]]
if {[lindex $_reponame end] eq {.git}} {
@@ -1208,7 +1210,6 @@ if {[lindex $_reponame end] eq {.git}} {
}
set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
######################################################################
##
@@ -2007,7 +2008,7 @@ proc incr_font_size {font {amt 1}} {
proc do_gitk {revs {is_submodule false}} {
global current_diff_path file_states current_diff_side ui_index
- global _gitdir _gitworktree
+ global _gitdir
# -- Always start gitk through whatever we were loaded with. This
# lets us bypass using shell process on Windows systems.
@@ -2041,18 +2042,16 @@ proc do_gitk {revs {is_submodule false}} {
}
set revs $old_sha1...$new_sha1
}
- # GIT_DIR and GIT_WORK_TREE for the submodule are not the ones
- # we've been using for the main repository, so unset them.
+ # GIT_DIR for the submodule is not the one we've been using for
+ # the main repository, so unset it. (GIT_WORK_TREE is already unset.)
# TODO we could make life easier (start up faster?) for gitk
# by setting these to the appropriate values to allow gitk
# to skip the heuristics to find their proper value
unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
}
safe_exec_bg [concat $cmd $revs "--" "--"]
set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
cd $pwd
if {[info exists main_status]} {
@@ -2076,12 +2075,11 @@ proc do_git_gui {} {
error_popup [mc "Couldn't find git gui in PATH"]
} else {
global env
- global _gitdir _gitworktree
+ global _gitdir
- # see note in do_gitk about unsetting these vars when
+ # see note in do_gitk about unsetting this variable when
# running tools in a submodule
unset env(GIT_DIR)
- unset env(GIT_WORK_TREE)
set pwd [pwd]
cd $current_diff_path
@@ -2089,7 +2087,6 @@ proc do_git_gui {} {
safe_exec_bg [concat $exe gui]
set env(GIT_DIR) $_gitdir
- set env(GIT_WORK_TREE) $_gitworktree
cd $pwd
set status_operation [$::main_status \
--
2.54.0.215.g4fe990ec16
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH v2 02/11] git-gui: return status from choose_repository::pick
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-22 8:18 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 03/11] git-gui: use --absolute-git-dir Mark Levedahl
` (8 subsequent siblings)
10 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
The repository picker (choose_repository::pick) on success always
returns with the current directory at the root of the selected worktree,
and with the global variable _gitdir holding the name of the git
repository, possibly as a relative path. On failure, _gitdir = {}. If
the selection was from the "recent" list, no validation has occurred.
There are too many side effects in this interface. Note that the picker
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. So, let's change pick to return a 0/1 value, 1
meaning a worktreee + repo was selected and the current directory is the
worktree root, and leave validation and setting of _gitdir,
_gitworktree, and _prefix to the caller. Note: pick actually does not
return if something was not selected, rather it terminates git-gui.
But, let's pretend at the call site that pick returns 0/false instead.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 10 ++++++++--
lib/choose_repository.tcl | 21 ++++++++-------------
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 4ba25da7b6..4a736190a9 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1151,10 +1151,16 @@ if {[catch {
} err]} {
load_config 1
apply_config
- choose_repository::pick
- if {![file isdirectory $_gitdir]} {
+ if {![choose_repository::pick]} {
exit 1
}
+ if {[catch {
+ set _gitdir [git rev-parse --git-dir]
+ } err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
+ }
+ set _prefix {}
set picked 1
}
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index 7e1462a20c..4b06afee93 100644
--- 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
}
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v2 02/11] git-gui: return status from choose_repository::pick
2026-05-20 20:24 ` [PATCH v2 02/11] git-gui: return status from choose_repository::pick Mark Levedahl
@ 2026-05-22 8:18 ` Johannes Sixt
0 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-22 8:18 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> The repository picker (choose_repository::pick) on success always
> returns with the current directory at the root of the selected worktree,
> and with the global variable _gitdir holding the name of the git
> repository, possibly as a relative path. On failure, _gitdir = {}. If
> the selection was from the "recent" list, no validation has occurred.
>
> There are too many side effects in this interface. Note that the picker
> 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. So, let's change pick to return a 0/1 value, 1
> meaning a worktreee + repo was selected and the current directory is the
> worktree root, and leave validation and setting of _gitdir,
> _gitworktree, and _prefix to the caller.
While the removal of side-effects from the picker is very much desired,
the new return value sounds over-engineered at this point, in particular
due to this note:
> Note: pick actually does not
> return if something was not selected, rather it terminates git-gui.
> But, let's pretend at the call site that pick returns 0/false instead.
If we need the return value later, let's postpone that part of this
commit until then.
> diff --git a/git-gui.sh b/git-gui.sh
> index 4ba25da7b6..4a736190a9 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1151,10 +1151,16 @@ if {[catch {
> } err]} {
> load_config 1
> apply_config
> - choose_repository::pick
> - if {![file isdirectory $_gitdir]} {
> + if {![choose_repository::pick]} {
> exit 1
> }
> + if {[catch {
> + set _gitdir [git rev-parse --git-dir]
> + } err]} {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
There's something wrong with the quotes here, and an 'exit 1' is missing.
> + }
> + set _prefix {}
> set picked 1
> }
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v2 03/11] git-gui: use --absolute-git-dir
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 01/11] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 02/11] git-gui: return status from choose_repository::pick Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-22 8:25 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
` (7 subsequent siblings)
10 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git-gui uses git rev-parse --git-dir to get the pathname of the
discovered git repository. The returned value can be relative, and is
'.' if the current directory is the top of the repository directory
itself. git-gui has code to change '.' to [pwd] in this case so that
subsequent logic runs.
But, git rev-parse supports --absolute-git-dir from fac60b8925
("rev-parse: add option for absolute or relative path formatting",
2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
this more useful form is always available. Use --absolute-git-dir to
always get an absolute path, avoiding the need for other checks, and
delete the now unneeded code to fix a relative _gitdir.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 4a736190a9..233c975786 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1146,7 +1146,7 @@ if {[catch {
&& [catch {
# beware that from the .git dir this sets _gitdir to .
# and _prefix to the empty string
- set _gitdir [git rev-parse --git-dir]
+ set _gitdir [git rev-parse --absolute-git-dir]
set _prefix [git rev-parse --show-prefix]
} err]} {
load_config 1
@@ -1155,7 +1155,7 @@ if {[catch {
exit 1
}
if {[catch {
- set _gitdir [git rev-parse --git-dir]
+ set _gitdir [git rev-parse --absolute-git-dir]
} err]} {
catch {wm withdraw .}
error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
@@ -1175,13 +1175,6 @@ if {$hashalgorithm eq "sha1"} {
exit 1
}
-# we expand the _gitdir when it's just a single dot (i.e. when we're being
-# run from the .git dir itself) lest the routines to find the worktree
-# get confused
-if {$_gitdir eq "."} {
- set _gitdir [pwd]
-}
-
if {![file isdirectory $_gitdir]} {
catch {wm withdraw .}
error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v2 03/11] git-gui: use --absolute-git-dir
2026-05-20 20:24 ` [PATCH v2 03/11] git-gui: use --absolute-git-dir Mark Levedahl
@ 2026-05-22 8:25 ` Johannes Sixt
0 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-22 8:25 UTC (permalink / raw)
To: Mark Levedahl, git; +Cc: egg_mushroomcow, bootaina702
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git-gui uses git rev-parse --git-dir to get the pathname of the
> discovered git repository. The returned value can be relative, and is
> '.' if the current directory is the top of the repository directory
> itself. git-gui has code to change '.' to [pwd] in this case so that
> subsequent logic runs.
>
> But, git rev-parse supports --absolute-git-dir from fac60b8925
> ("rev-parse: add option for absolute or relative path formatting",
> 2020-12-13), and included in git 2.31. git-gui requires git >= 2.36, so
> this more useful form is always available. Use --absolute-git-dir to
> always get an absolute path, avoiding the need for other checks, and
> delete the now unneeded code to fix a relative _gitdir.
Very good!
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 4a736190a9..233c975786 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1146,7 +1146,7 @@ if {[catch {
> && [catch {
> # beware that from the .git dir this sets _gitdir to .
> # and _prefix to the empty string
Note that the comment above needs some adjustment as well.
> - set _gitdir [git rev-parse --git-dir]
> + set _gitdir [git rev-parse --absolute-git-dir]
> set _prefix [git rev-parse --show-prefix]
> } err]} {
> load_config 1
> @@ -1155,7 +1155,7 @@ if {[catch {
> exit 1
> }
> if {[catch {
> - set _gitdir [git rev-parse --git-dir]
> + set _gitdir [git rev-parse --absolute-git-dir]
> } err]} {
> catch {wm withdraw .}
> error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
> @@ -1175,13 +1175,6 @@ if {$hashalgorithm eq "sha1"} {
> exit 1
> }
>
> -# we expand the _gitdir when it's just a single dot (i.e. when we're being
> -# run from the .git dir itself) lest the routines to find the worktree
> -# get confused
> -if {$_gitdir eq "."} {
> - set _gitdir [pwd]
> -}
> -
> if {![file isdirectory $_gitdir]} {
> catch {wm withdraw .}
> error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (2 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 03/11] git-gui: use --absolute-git-dir Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-22 8:46 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 05/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
` (6 subsequent siblings)
10 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git-gui attempts to use env(GIT_DIR) directly as the git repository,
accepting GIT_DIR if it is a directory. Only if that fails is git
rev-parse used to discover the repository. But, this avoids all of
git-core's validity checking on a repository, thus possibly deferring an
error to a later step, possibly unexpected. Repository validation should
be part of initial setup so that later processing does not need error
trapping for configuration errors.
Let's just invoke rev-parse so all error checking is done.
While here, let's cleanup the error handling.
Stop if an error occurs and the user set GIT_DIR or GIT_WORK_TREE.
Use of either or both of those variables is supported by git, but their
use also means the user has taken responsibility that they are correct,
so a failure is something the user must address.
Otherwise on error, continue the existing behavior and show the
repository picker. But, let's move the possible invocation of
repository_chooser::pick to a separate code block. This permits adding
separate conditions on using pick indepent of repository discovery, and
will be exploited later in the series. Note that the picker always
returns with the current directory in the root of a worktree with the
git repository is in the .git subdirectory. The variable "picked" is
used by git-gui to automatically execute the "Explore Working Copy" menu
item after the repository picker is run. This is controlled by config
variable gui.autoexplore, and happens after all discovery is complete.
Remove a later check on whether _gitdir is a directory: that code
cannot be reached without rev-parse already validating the repository.
_prefix should not be set before worktree discovery: the prefix is only
known after the worktree is found, and at this point we have only
discovered the repository. This is true even when running the repository
picker: that option provides a list of prior selections, and does no
validation on the list beyond checking that the directories exist. For
now, just initialize _prefix along with other global variables.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 48 +++++++++++++++++++++++++++++++++---------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 233c975786..c61a6cbd8f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -374,6 +374,7 @@ set _gitdir {}
set _gitworktree {}
set _isbare {}
set _githtmldir {}
+set _prefix {}
set _reponame {}
set _shellpath {@@SHELL_PATH@@}
@@ -1122,6 +1123,24 @@ unset argv0dir
##
## repository setup
+proc is_gitvars_error {err} {
+ set havevars 0
+ set GIT_DIR {}
+ set GIT_WORK_TREE {}
+ catch {set GIT_DIR $::env(GIT_DIR); set havevars 1}
+ catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE) ; set havevars 1}
+
+ if {$havevars} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Invalid configuration:"] \
+ "\n" "GIT_DIR: " $GIT_DIR \
+ "\n" "GIT_WORK_TREE: " $GIT_WORK_TREE \
+ "\n\n$err"]
+ return 1
+ }
+ return 0
+}
+
proc set_gitdir_vars {} {
global _gitdir _gitworktree env
if {$_gitdir ne {}} {
@@ -1138,17 +1157,22 @@ proc unset_gitdir_vars {} {
catch {unset env(GIT_WORK_TREE)}
}
-set picked 0
-if {[catch {
- set _gitdir $env(GIT_DIR)
- set _prefix {}
- }]
- && [catch {
- # beware that from the .git dir this sets _gitdir to .
- # and _prefix to the empty string
+# find repository.
+set _gitdir {}
+if {$_gitdir eq {}} {
+ if {[catch {
set _gitdir [git rev-parse --absolute-git-dir]
- set _prefix [git rev-parse --show-prefix]
} err]} {
+ if {[is_gitvars_error $err]} {
+ exit 1
+ }
+ set _gitdir {}
+ }
+}
+
+set picked 0
+if {$_gitdir eq {}} {
+ unset_gitdir_vars
load_config 1
apply_config
if {![choose_repository::pick]} {
@@ -1160,7 +1184,6 @@ if {[catch {
catch {wm withdraw .}
error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
}
- set _prefix {}
set picked 1
}
@@ -1175,11 +1198,6 @@ if {$hashalgorithm eq "sha1"} {
exit 1
}
-if {![file isdirectory $_gitdir]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
- exit 1
-}
# _gitdir exists, so try loading the config
load_config 0
apply_config
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository
2026-05-20 20:24 ` [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
@ 2026-05-22 8:46 ` Johannes Sixt
0 siblings, 0 replies; 96+ messages in thread
From: Johannes Sixt @ 2026-05-22 8:46 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
Am 20.05.26 um 22:24 schrieb Mark Levedahl:
> git-gui attempts to use env(GIT_DIR) directly as the git repository,
> accepting GIT_DIR if it is a directory. Only if that fails is git
> rev-parse used to discover the repository. But, this avoids all of
> git-core's validity checking on a repository, thus possibly deferring an
> error to a later step, possibly unexpected. Repository validation should
> be part of initial setup so that later processing does not need error
> trapping for configuration errors.
>
> Let's just invoke rev-parse so all error checking is done.
>
> While here, let's cleanup the error handling.
>
> Stop if an error occurs and the user set GIT_DIR or GIT_WORK_TREE.
> Use of either or both of those variables is supported by git, but their
> use also means the user has taken responsibility that they are correct,
> so a failure is something the user must address.
Very much so!
>
> Otherwise on error, continue the existing behavior and show the
> repository picker. But, let's move the possible invocation of
> repository_chooser::pick to a separate code block. This permits adding
> separate conditions on using pick indepent of repository discovery, and
> will be exploited later in the series. Note that the picker always
> returns with the current directory in the root of a worktree with the
> git repository is in the .git subdirectory. The variable "picked" is
> used by git-gui to automatically execute the "Explore Working Copy" menu
> item after the repository picker is run. This is controlled by config
> variable gui.autoexplore, and happens after all discovery is complete.
>
> Remove a later check on whether _gitdir is a directory: that code
> cannot be reached without rev-parse already validating the repository.
>
> _prefix should not be set before worktree discovery: the prefix is only
> known after the worktree is found, and at this point we have only
> discovered the repository.
Sorry, but I cannot agree with "prefix is only known after the worktree
is found". The prefix is a property that can be known even if we haven't
asked where the top-level of the working tree is. See more below.
> This is true even when running the repository
> picker: that option provides a list of prior selections, and does no
> validation on the list beyond checking that the directories exist. For
> now, just initialize _prefix along with other global variables.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> git-gui.sh | 48 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 233c975786..c61a6cbd8f 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -374,6 +374,7 @@ set _gitdir {}
> set _gitworktree {}
> set _isbare {}
> set _githtmldir {}
> +set _prefix {}
> set _reponame {}
> set _shellpath {@@SHELL_PATH@@}
>
> @@ -1122,6 +1123,24 @@ unset argv0dir
> ##
> ## repository setup
>
> +proc is_gitvars_error {err} {
> + set havevars 0
> + set GIT_DIR {}
> + set GIT_WORK_TREE {}
> + catch {set GIT_DIR $::env(GIT_DIR); set havevars 1}
> + catch {set GIT_WORK_TREE $::env(GIT_WORK_TREE) ; set havevars 1}
> +
> + if {$havevars} {
> + catch {wm withdraw .}
> + error_popup [strcat [mc "Invalid configuration:"] \
> + "\n" "GIT_DIR: " $GIT_DIR \
> + "\n" "GIT_WORK_TREE: " $GIT_WORK_TREE \
> + "\n\n$err"]
> + return 1
> + }
> + return 0
> +}
> +
> proc set_gitdir_vars {} {
> global _gitdir _gitworktree env
> if {$_gitdir ne {}} {
> @@ -1138,17 +1157,22 @@ proc unset_gitdir_vars {} {
> catch {unset env(GIT_WORK_TREE)}
> }
>
> -set picked 0
> -if {[catch {
> - set _gitdir $env(GIT_DIR)
> - set _prefix {}
> - }]
> - && [catch {
> - # beware that from the .git dir this sets _gitdir to .
> - # and _prefix to the empty string
> +# find repository.
> +set _gitdir {}
> +if {$_gitdir eq {}} {
> + if {[catch {
> set _gitdir [git rev-parse --absolute-git-dir]
> - set _prefix [git rev-parse --show-prefix]
You cannot leave the _prefix empty, because it breaks `git gui browser
master dir` when invoked from a subdirectory of the working tree.
This line must remain. I see that you add it back in later patch. There
may be some motivation to move prefix discovery, but there is no
motivation to remove it at this point.
> } err]} {
> + if {[is_gitvars_error $err]} {
> + exit 1
> + }
> + set _gitdir {}
BTW, this line would only be needed if the 'set _prefix' line above stays.
> + }
> +}
> +
> +set picked 0
> +if {$_gitdir eq {}} {
> + unset_gitdir_vars
> load_config 1
> apply_config
> if {![choose_repository::pick]} {
> @@ -1160,7 +1184,6 @@ if {[catch {
> catch {wm withdraw .}
> error_popup [strcat [mc "Unusable repo/worktree:"] " [pwd] "\n\n$err"]
> }
> - set _prefix {}
> set picked 1
> }
>
> @@ -1175,11 +1198,6 @@ if {$hashalgorithm eq "sha1"} {
> exit 1
> }
>
> -if {![file isdirectory $_gitdir]} {
> - catch {wm withdraw .}
> - error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
> - exit 1
> -}
> # _gitdir exists, so try loading the config
> load_config 0
> apply_config
-- Hannes
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v2 05/11] git-gui: simplify [is_bare] to report if a worktree is known
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (3 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 04/11] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery Mark Levedahl
` (5 subsequent siblings)
10 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git-gui includes proc is_bare, used in several places to make decisions
on whether a worktree exists, but also in discovery to tell if a
worktree can be supported.
But, is_bare is out of date with regard to multiple worktrees, safe
repository guards, and possibly other relevant features known to git
rev-parse. Also, is_bare caches its result on the first call, so is not
useful if a later step in the discovery process finds a worktree.
So, simplify is_bare to report whether git-gui has a worktree or is
working only from a repository.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index c61a6cbd8f..936c309e59 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -372,7 +372,6 @@ if {[tk windowingsystem] eq "aqua"} {
set _appname {Git Gui}
set _gitdir {}
set _gitworktree {}
-set _isbare {}
set _githtmldir {}
set _prefix {}
set _reponame {}
@@ -524,29 +523,7 @@ proc get_config {name} {
}
proc is_bare {} {
- global _isbare
- global _gitdir
- global _gitworktree
-
- if {$_isbare eq {}} {
- if {[catch {
- set _bare [git rev-parse --is-bare-repository]
- switch -- $_bare {
- true { set _isbare 1 }
- false { set _isbare 0}
- default { throw }
- }
- }]} {
- if {[is_config_true core.bare]
- || ($_gitworktree eq {}
- && [lindex [file split $_gitdir] end] ne {.git})} {
- set _isbare 1
- } else {
- set _isbare 0
- }
- }
- }
- return $_isbare
+ return [expr {$::_gitworktree eq {}}]
}
######################################################################
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (4 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 05/11] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir Mark Levedahl
` (4 subsequent siblings)
10 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git gui uses a combination of tcl code and git invocations to determine
the worktree and the location with respect to the worktree root
(_prefix). But, git rev-parse provides all of this information directly,
and assures full error and configuration checking are done by git
itself. The entirety of discovery in normal configurations involves
git rev-parse --show-toplevel (gets worktree root)
git rev-parse --show-prefix (shows location wrt the root)
An error thrown on either of these lines means the worktree discovered
by git is unusable, or git did not discover a worktree because the
current directory is inside the repository. If the user has defined
GIT_DIR or GIT_WORK_TREE, this is a user configuration error and git-gui
should stop.
Otherwise, the blame or browser subcommands can be used without a
worktree.
A separate error might occur when changing to the root of the discovered
worktree. The cause would be file system related and completely outside
of git's control. So, the final "cd $worktree_root" is separately
trapped.
Discovery of the repository and the worktree must be guarded to trap
errors: the intent is that any configuration problems are caught during
discovery, and later processing need not include error trapping and
recovery. So, move all worktree discovery code to be immediately after
repository discovery.
This does move configuration loading to occur after worktree discovery
rather than before. None of the code executed in worktree discovery has
any option controlled by a git-gui configuration variable, so no impact
is expected. git itself will always read the repository configuration,
including worktree specific configuration data if that exists, so this
is unaffected by when git-gui loads its own config data, and we cannot
be sure the full worktree dependent configuration can be loaded before
full discovery is complete.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 64 +++++++++++++++++++++++++-----------------------------
1 file changed, 30 insertions(+), 34 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 936c309e59..8fe25fe188 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1164,6 +1164,36 @@ if {$_gitdir eq {}} {
set picked 1
}
+# find worktree, continue without if not required
+if {[catch {
+ set _gitworktree [git rev-parse --show-toplevel]
+ set _prefix [git rev-parse --show-prefix]
+} err]} {
+ if {[is_gitvars_error $err]} {
+ exit 1
+ }
+ set _gitworktree {}
+ set _prefix {}
+}
+
+if {![is_bare]} {
+ if {[catch {
+ cd $_gitworktree
+ } err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot change to discovered worktree: "] \
+ "$_gitworktree" "\n\n$err"]
+ exit 1;
+ }
+} elseif {![is_enabled bare]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" $_gitdir]
+ exit 1
+}
+
+# repository and worktree config are complete, export them
+set_gitdir_vars
+
# Use object format as hash algorithm (either "sha1" or "sha256")
set hashalgorithm [git rev-parse --show-object-format]
if {$hashalgorithm eq "sha1"} {
@@ -1179,37 +1209,6 @@ if {$hashalgorithm eq "sha1"} {
load_config 0
apply_config
-set _gitworktree [git rev-parse --show-toplevel]
-
-if {$_prefix ne {}} {
- if {$_gitworktree eq {}} {
- regsub -all {[^/]+/} $_prefix ../ cdup
- } else {
- set cdup $_gitworktree
- }
- if {[catch {cd $cdup} err]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
- exit 1
- }
- set _gitworktree [pwd]
- unset cdup
-} elseif {![is_enabled bare]} {
- if {[is_bare]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
- exit 1
- }
- if {$_gitworktree eq {}} {
- set _gitworktree [file dirname $_gitdir]
- }
- if {[catch {cd $_gitworktree} err]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
- exit 1
- }
- set _gitworktree [pwd]
-}
set _reponame [file split [file normalize $_gitdir]]
if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end-1]
@@ -1217,9 +1216,6 @@ if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end]
}
-# Export the final paths
-set_gitdir_vars
-
######################################################################
##
## global init
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (5 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-21 4:55 ` Shroom Moo
2026-05-20 20:24 ` [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix) Mark Levedahl
` (3 subsequent siblings)
10 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git-gui, since 87cd09f43e ("git-gui: work from the .git dir",
2010-01-23), has had the intent to allow starting from inside a
repository, then switching to the parent directory if that is a valid
worktree.
This certainly hasn't worked since 2d92ab32fd ("rev-parse: make
--show-toplevel without a worktree an error", 2019-11-19) in git, but
breaking this git-gui feature was unintentional.
There are (at least) 3 cases where the gitdir can tell us where the
worktree is, and we would like all to work:
- core.worktree is set, and points to a valid worktree. This is already
handled by git rev-parse --show-toplevel, even when not in the worktree.
There is nothing more to do in this case.
- the gitdir is embedded in a worktree as subdirectory .git. The parent
is (or at least should be) a valid worktree. This worked long ago.
- the gitdir is a worktree specific directory (under
<mainrepo>/worktrees/worktree_name), within which there is a file
"gitdir" pointing to .git in the worktree. git gui never learned to
handle this case.
Let's handle the latter two cases. Always check that the discovered
worktree is valid and points to the already discovered gitdir according
to git rev-parse. This avoids issues that may arise because we are
discovering from the gitdir up, rather than the worktree down, and file
system non-posix behavior or misconfiguration of git might cause
confusion. For instance, a manually moved worktree might not be where
the gitdir points.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index 8fe25fe188..aeb7ed3548 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1100,6 +1100,41 @@ unset argv0dir
##
## repository setup
+proc find_worktree_from_gitdir {} {
+ # Directory 'parent' of a repository named 'parent/.git' might be the worktree.
+ # Assure parent is a worktree and using the git repository already discovered.
+ # Also, handle case of being in a worktree's gitdir, where file "gitdir" points to
+ # gitlink file .git in the real worktree.
+ set worktree {}
+ if {[file tail $::_gitdir] eq {.git}} {
+ if {[catch {
+ set gitdir_parent [file dirname $::_gitdir]
+ set worktree [git -C $gitdir_parent rev-parse --show-toplevel]
+ set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
+ if {$::_gitdir ne $parent_gitdir} {
+ set worktree {}
+ }
+ }]} {
+ set worktree {}
+ }
+ } elseif [file exists {gitdir}] {
+ if {[catch {
+ set fd_gitdir [open {gitdir} {r}]
+ set gitlink_parent [file dirname [read $fd_gitdir]]
+ catch {close $fd_gitdir}
+ set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
+ set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
+ if {$::_gitdir ne $parent_gitdir} {
+ set worktree {}
+ }
+ }]} {
+ catch {close $fd_gitdir}
+ set worktree {}
+ }
+ }
+ return $worktree
+}
+
proc is_gitvars_error {err} {
set havevars 0
set GIT_DIR {}
@@ -1176,6 +1211,13 @@ if {[catch {
set _prefix {}
}
+if {[is_bare]} {
+ # Maybe we are in an embedded or worktree specific gitdir
+ if {[set _gitworktree [find_worktree_from_gitdir]] ne {}} {
+ set _prefix {}
+ }
+}
+
if {![is_bare]} {
if {[catch {
cd $_gitworktree
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
2026-05-20 20:24 ` [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir Mark Levedahl
@ 2026-05-21 4:55 ` Shroom Moo
2026-05-21 17:45 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Shroom Moo @ 2026-05-21 4:55 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git, Johannes Sixt, Aina Boot
On 5/21/26 4:24 AM, Mark Levedahl wrote:
> + } elseif [file exists {gitdir}] {
> + if {[catch {
> + set fd_gitdir [open {gitdir} {r}]
> + set gitlink_parent [file dirname [read $fd_gitdir]]
> + catch {close $fd_gitdir}
> + set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
> + set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
> + if {$::_gitdir ne $parent_gitdir} {
> + set worktree {}
> + }
> + }]} {
> + catch {close $fd_gitdir}
> + set worktree {}
> + }
> + }
There is also an unaddressed issue:
In [file exists {gitdir}] and [open {gitdir} r], {gitdir} is a
literal string referring to a file named gitdir in the current
working directory. However, in the context of a linked worktree
(created via git worktree add), the actual file path is
$_gitdir/gitdir (e.g., .git/worktrees/<name>/gitdir). While the
current working directory could be anywhere (even inside the .git
directory), $_gitdir is an absolute path pointing to that worktree's
gitdir (e.g., /path/to/main/.git/worktrees/branch). The gitdir file
resides within the $_gitdir directory and contains a relative path
like ../../.git/worktrees/branch. The current code logic will never
locate this file.
Additionally, [file exists {gitdir}] checks for the gitdir file in
the current working directory. Since the function has not yet
switched to $_gitdir when this check runs, it is almost impossible
to find the file. Consequently, this logic never triggers, preventing
linked worktrees from being recognized.
Maybe the identification of linked worktree should not directly look
for the gitdir file, but should check whether there is a.git file and
its content points to... /.git/worktrees/... ? Anyways, using the
literal {gitdir} to search in the current directory lead to risks.
Shroom
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
2026-05-21 4:55 ` Shroom Moo
@ 2026-05-21 17:45 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-21 17:45 UTC (permalink / raw)
To: Shroom Moo; +Cc: git, Johannes Sixt, Aina Boot
On 5/21/26 12:55 AM, Shroom Moo wrote:
> On 5/21/26 4:24 AM, Mark Levedahl wrote:
>> + } elseif [file exists {gitdir}] {
>> + if {[catch {
>> + set fd_gitdir [open {gitdir} {r}]
>> + set gitlink_parent [file dirname [read $fd_gitdir]]
>> + catch {close $fd_gitdir}
>> + set worktree [git -C $gitlink_parent rev-parse --show-toplevel]
>> + set parent_gitdir [git -C $worktree rev-parse --absolute-git-dir]
>> + if {$::_gitdir ne $parent_gitdir} {
>> + set worktree {}
>> + }
>> + }]} {
>> + catch {close $fd_gitdir}
>> + set worktree {}
>> + }
>> + }
> There is also an unaddressed issue:
> In [file exists {gitdir}] and [open {gitdir} r], {gitdir} is a
> literal string referring to a file named gitdir in the current
> working directory. However, in the context of a linked worktree
> (created via git worktree add), the actual file path is
> $_gitdir/gitdir (e.g., .git/worktrees/<name>/gitdir). While the
> current working directory could be anywhere (even inside the .git
> directory), $_gitdir is an absolute path pointing to that worktree's
> gitdir (e.g., /path/to/main/.git/worktrees/branch). The gitdir file
> resides within the $_gitdir directory and contains a relative path
> like ../../.git/worktrees/branch. The current code logic will never
> locate this file.
You have to be in the particular worktree's gitdir for this to work. I there exists
worktrees/foo
worktrees/frotz
worktrees/bar
Which would we expore? The code above must be in foo, frotz, bar
The main worktree is found not from worktrees/*, but from the root of the gitdir.
>
> Additionally, [file exists {gitdir}] checks for the gitdir file in
> the current working directory. Since the function has not yet
> switched to $_gitdir when this check runs, it is almost impossible
> to find the file. Consequently, this logic never triggers, preventing
> linked worktrees from being recognized.
>
> Maybe the identification of linked worktree should not directly look
> for the gitdir file, but should check whether there is a.git file and
> its content points to... /.git/worktrees/... ? Anyways, using the
> literal {gitdir} to search in the current directory lead to risks.
>
> Shroom
>
We cannot get to this code if not inside the gitdir, and if the user set GIT_DIR and/or
GIT_WORK_TREE to do something clever, that either worked or the code already threw an
error. git, without GIT_WORK_TREE set, uses the current directory as the worktree, or the
parent directory containing .git. So, we must be inside the gitdir if this code path gets hit.
Mark
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix)
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (6 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
` (2 subsequent siblings)
10 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
commit f87a36b697 ("git-gui: use git-branch --show-current", 2024-02-12)
changed git-gui to use git-branch to access refs, rather than directly
reading files as doing the latter is not compatible with the reftable
backend. git branch --show-current reports an empty branch name when the
head is detached, and in this case load_current_branch needs to report
HEAD using special case logic as it did prior to the above commit. Make
it do so.
This addresses an issue with git-gui browser failing with a detached
head.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index aeb7ed3548..a72d8a59ec 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -648,6 +648,9 @@ proc load_current_branch {} {
set current_branch [git branch --show-current]
set is_detached [expr [string length $current_branch] == 0]
+ if {$is_detached} {
+ set current_branch {HEAD}
+ }
}
auto_load tk_optionMenu
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (7 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix) Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
10 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
Invoking "git-gui browser rev ." should show the file browser for the
commitish rev, starting at the current directory. When the current
directory is the working tree root, this errors out in normalize_relpath
because the '.' is removed, yielding an empty list as argument to [file
join ...]. The browser function demands "./" in this case, so make it
so. (./ works on Windows as well because g4w accepts posix file
naming).
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/git-gui.sh b/git-gui.sh
index a72d8a59ec..d373457901 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3007,7 +3007,11 @@ proc normalize_relpath {path} {
}
lappend elements $item
}
- return [eval file join $elements]
+ if {$elements ne {}} {
+ return [eval file join $elements]
+ } else {
+ return {./}
+ }
}
# -- Not a normal commit type invocation? Do that instead!
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (8 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
2026-05-21 5:02 ` Shroom Moo
2026-05-20 20:24 ` [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
10 siblings, 1 reply; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
git-gui's blame and browser subcommands do not work with bare
repositories, but they should per commit c52c94524b ("git-gui: Allow
blame/browser subcommands on bare repositories", 2007-07-17). Assuming
that commit worked, something changed since reintroducing a hard-coded
dependency upon a worktree.
The basic issue goes back to 3e45ee1ef2 ("git-gui: Smarter command line
parsing for browser, blame", 2007-05-08), which seeks to implement
command line parsing similar to git blame. That commit introduces
depencies upon the worktree to decide which argument is rev or path.
Looking at builtin/blame.c in git around line 1120:
* (1) if dashdash_pos != 0, it is either
* "blame [revisions] -- <path>" or
* "blame -- <path> <rev>"
*
* (2) otherwise, it is one of the two:
* "blame [revisions] <path>"
* "blame <path> <rev>"
shows the clear intent: rev and path may be swapped in input so both
meanings must be tried, but -- may be used to designate which is the
path forcing or precluding trying the swapped arguments.
With a worktree, git gui correctly swaps the arguments if the given path
exists in the worktree. git blame does this using the git repository.
But, git-gui sometimes interprets the -- to have an exactly opposite
meaning:
git blame Makefile gitgui-0.19.0 works
git gui blame Makefile gitgui-0.19.0 works
git blame -- Makefile gitgui-0.19.0 works
git gui blame -- Makefile gitgui-0.19.0 works
git blame Makefile -- gitgui-0.19.0 fails (correctly)
git gui blame Makefile -- gitgui-0.19.0 works (should fail)
git blame gitgui-0.19.0 -- Makefile works (correctly)
git gui blame gitgui-0.19.0 -- Makefile fails (should work)
It is possible to patch the code to operate without a worktree, but this
will make the commands operate differently with and without a worktree,
won't fix the parsing issues above, and won't address the issues that
can arise when using a worktree to help decisions on a different rev
with file/directory conflicts, etc.
So, let's rework the parser so that it uses -- as does git blame, and
uses git ls-tree to query the given revision for existence and type of
path rather than basing this upon a possibly unrelated worktree. Also,
abort early when the given path is not found, or does not match the need
(file or directory). This fixes some current cases where git-gui will
open a window with no content, possibly also with an error message.
This does not change whether or how git-gui uses staged and unstaged
content in the current worktree for blame display.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 151 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 87 insertions(+), 64 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index d373457901..ae609f86f1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3014,100 +3014,123 @@ proc normalize_relpath {path} {
}
}
+proc find_path_type {head path} {
+ if {$path eq {./}} {
+ # the root-tree exists in every rev, ls-tree gives data on the contents,
+ # not the type of tree itself. So, if the rev exists, return {tree}
+ if {[catch {set objtype [git ls-tree $head]}]} {
+ set objtype {}
+ } else {
+ set objtype {tree}
+ }
+ } else {
+ # test that the path exists in head, ls-tree gives info on the path only
+ if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
+ set objtype {}
+ }
+ }
+ return $objtype
+}
+
# -- Not a normal commit type invocation? Do that instead!
#
switch -- $subcommand {
browser -
blame {
if {$subcommand eq "blame"} {
- set subcommand_args {[--line=<num>] rev? path}
+ set subcommand_args {[--line=<num>] <[rev] [--] filename | [--] filename rev>}
+ set required_objtype blob
} else {
- set subcommand_args {rev? path}
+ set subcommand_args {<[rev] [--] directory | [--] directory rev>}
+ set required_objtype tree
}
- if {$argv eq {}} usage
+ set maxargs [llength $subcommand_args]
+ set nargs [llength $argv]
+ if {$nargs < 1 || $nargs > $maxargs} usage
set head {}
+ set althead {}
set path {}
+ set altpath {}
+ set canswap 1
set jump_spec {}
- set is_path 0
- foreach a $argv {
- set p [file join $_prefix $a]
- if {$is_path || [file exists $p]} {
- if {$path ne {}} usage
- set path [normalize_relpath $p]
- break
- } elseif {$a eq {--}} {
- if {$path ne {}} {
- if {$head ne {}} usage
- set head $path
- set path {}
+ # assume: [--line=num] [head] [--] path as the possible arguments, in order.
+ # head and path may need a swap later.
+ for {set iarg 0} {$iarg < $nargs} {incr iarg} {
+ set arg [lindex $argv $iarg]
+ if {$arg eq {--}} {
+ # next arg is the path, prevent or FORCE swap?
+ if {$iarg == $nargs - 2} {
+ set canswap 0
+ } elseif {$iarg == $nargs - 3} {
+ set canswap 2
+ } else {
+ usage
}
- set is_path 1
- } elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
- if {$jump_spec ne {} || $head ne {}} usage
+ } elseif {[regexp {^--line=(\d+)$} $arg arg lnum]} {
+ # --line can only be the first arg
+ if {$iarg != 0 || $maxargs < 4} usage
set jump_spec [list $lnum]
+ } elseif {$iarg == $nargs - 1} {
+ # assume final argument is path
+ set path [normalize_relpath [file join $_prefix $arg]]
+ set althead $arg
} elseif {$head eq {}} {
- if {$head ne {}} usage
- set head $a
- set is_path 1
+ # assume the other argument is head
+ set head $arg
+ set altpath [normalize_relpath [file join $_prefix $arg]]
} else {
usage
}
}
- unset is_path
-
- if {$head ne {} && $path eq {}} {
- if {[string index $head 0] eq {/}} {
- set path [normalize_relpath $head]
- set head {}
- } else {
- set path [normalize_relpath $_prefix$head]
- set head {}
- }
- }
+ # no swapping allowed if head not given, use current branch (HEAD)
if {$head eq {}} {
load_current_branch
- } else {
- if {[regexp [string map "@@ [expr $hashlength - 1]" {^[0-9a-f]{1,@@}$}] $head]} {
- if {[catch {
- set head [git rev-parse --verify $head]
- } err]} {
- if {[tk windowingsystem] eq "win32"} {
- tk_messageBox -icon error -title [mc Error] -message $err
- } else {
- puts stderr $err
- }
- exit 1
- }
+ set head $current_branch
+ set canswap 0
+ }
+
+ # -- before "rev" arg means we got -- path head
+ if {$canswap == 2} {
+ set head $althead
+ set path $altpath
+ set canswap 0
+ }
+
+ set objtype [find_path_type $head $path]
+ if {$objtype eq {} && $canswap} {
+ set objtype [find_path_type $althead $altpath]
+ if {$objtype ne {}} {
+ set head $althead
+ set path $altpath
}
- set current_branch $head
+ }
+ set current_branch $head
+
+ # check that path exists in head, and objtype matches need
+ if {$objtype ne $required_objtype} {
+ switch -- $required_objtype {
+ tree {set err [strcat \
+ [mc "'%s' is not a directory in rev '%s'" $path $head]]}
+ blob {set err [strcat \
+ [mc "'%s' is not a filename in rev '%s'" $path $head]]}
+ }
+ if {[tk windowingsystem] eq "win32"} {
+ catch {wm withdraw .}
+ error_popup $err
+ } else {
+ puts stderr $err
+ }
+ exit 1
}
wm deiconify .
switch -- $subcommand {
browser {
- if {$jump_spec ne {}} usage
- if {$head eq {}} {
- if {$path ne {} && [file isdirectory $path]} {
- set head $current_branch
- } else {
- set head $path
- set path {}
- }
- }
browser::new $head $path
}
- blame {
- if {$head eq {} && ![file exists $path]} {
- catch {wm withdraw .}
- tk_messageBox \
- -icon error \
- -type ok \
- -title [mc "git-gui: fatal error"] \
- -message [mc "fatal: cannot stat path %s: No such file or directory" $path]
- exit 1
- }
+ blame {
blame::new $head $path $jump_spec
}
}
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread* Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
2026-05-20 20:24 ` [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation Mark Levedahl
@ 2026-05-21 5:02 ` Shroom Moo
2026-05-21 17:35 ` Mark Levedahl
0 siblings, 1 reply; 96+ messages in thread
From: Shroom Moo @ 2026-05-21 5:02 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git, Johannes Sixt, Aina Boot
On 5/21/26 4:24 AM, Mark Levedahl wrote:
> +proc find_path_type {head path} {
> + if {$path eq {./}} {
> + # the root-tree exists in every rev, ls-tree gives data on the contents,
> + # not the type of tree itself. So, if the rev exists, return {tree}
> + if {[catch {set objtype [git ls-tree $head]}]} {
> + set objtype {}
> + } else {
> + set objtype {tree}
> + }
> + } else {
> + # test that the path exists in head, ls-tree gives info on the path only
> + if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
> + set objtype {}
> + }
> + }
> + return $objtype
> +}
In v1, argument parsing relied on file exists within the worktree to
determine if a path existed, without using ls-tree. In v2, the use of
git ls-tree seems to actually be intended to list directory contents,
rather than querying the type of the path itself.
If $path is a directory (a tree object), git ls-tree outputs the
object type for every entry within that directory, one per line.
The variable objtype is assigned a multi-line string. When compared
against "tree", the match fails, causing the function to return an
empty string, which subsequently leads to an error. We can change to
"git cat-file -t" or similiar approaches.
Shroom
^ permalink raw reply [flat|nested] 96+ messages in thread* Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
2026-05-21 5:02 ` Shroom Moo
@ 2026-05-21 17:35 ` Mark Levedahl
0 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-21 17:35 UTC (permalink / raw)
To: Shroom Moo; +Cc: git, Johannes Sixt, Aina Boot
On 5/21/26 1:02 AM, Shroom Moo wrote:
> On 5/21/26 4:24 AM, Mark Levedahl wrote:
>> +proc find_path_type {head path} {
>> + if {$path eq {./}} {
>> + # the root-tree exists in every rev, ls-tree gives data on the contents,
>> + # not the type of tree itself. So, if the rev exists, return {tree}
>> + if {[catch {set objtype [git ls-tree $head]}]} {
>> + set objtype {}
>> + } else {
>> + set objtype {tree}
>> + }
>> + } else {
>> + # test that the path exists in head, ls-tree gives info on the path only
>> + if {[catch {set objtype [git ls-tree {--format=%(objecttype)} $head $path]}]} {
>> + set objtype {}
>> + }
>> + }
>> + return $objtype
>> +}
> In v1, argument parsing relied on file exists within the worktree to
> determine if a path existed, without using ls-tree. In v2, the use of
> git ls-tree seems to actually be intended to list directory contents,
> rather than querying the type of the path itself.
>
> If $path is a directory (a tree object), git ls-tree outputs the
> object type for every entry within that directory, one per line.
>
> The variable objtype is assigned a multi-line string. When compared
> against "tree", the match fails, causing the function to return an
> empty string, which subsequently leads to an error. We can change to
> "git cat-file -t" or similiar approaches.
>
> Shroom
>
git ls-tree $rev $path --format='%(objecttype)' gives the type of the object at $path in
$rev, or an error. The types returned are "tree" for a directory, "blob" for a file. So
this gives definitive information if the object desired exists in the given rev, and is of
the right type. (We don't care about commits and tags, those cannot be blamed or browsed).
git-lstree $rev $dirname/ lists all of the objects in the directory, while git-lstree
$rev $dirname (no trailing /) gives info on the directory itself. There is no name for the
root directory itself, all of its contents are listed. That is why the root './' is
special case.
Asking the worktree that is on version 33 about whether frotz is a directory in version 2
is just asking for trouble, at best the worktree is authoritative for the checked out
version, but even then there can be uncommitted changed. In the root of git-gui, I get
/git-gui.sh browser gitgui-0.9.0 Makefile
'Makefile' is not a directory in rev 'gitgui-0.9.0'
so the types of objects are being checked.
Mark
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands
2026-05-20 20:23 ` [PATCH v2 " Mark Levedahl
` (9 preceding siblings ...)
2026-05-20 20:24 ` [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation Mark Levedahl
@ 2026-05-20 20:24 ` Mark Levedahl
10 siblings, 0 replies; 96+ messages in thread
From: Mark Levedahl @ 2026-05-20 20:24 UTC (permalink / raw)
To: git; +Cc: j6t, egg_mushroomcow, bootaina702, Mark Levedahl
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 discovery 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 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.
Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
git-gui.sh | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index ae609f86f1..299c1a0292 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1024,6 +1024,8 @@ proc load_config {include_global} {
##
## feature option selection
+enable_option picker
+enable_option gitdir_discovery
if {[regexp {^git-(.+)$} [file tail $argv0] _junk subcommand]} {
unset _junk
} else {
@@ -1035,6 +1037,9 @@ if {$subcommand eq {gui.sh}} {
if {$subcommand eq {gui} && [llength $argv] > 0} {
set subcommand [lindex $argv 0]
set argv [lrange $argv 1 end]
+ if {$subcommand eq {gui}} {
+ disable_option picker
+ }
}
enable_option multicommit
@@ -1050,6 +1055,7 @@ blame {
disable_option multicommit
disable_option branch
disable_option transport
+ disable_option picker
}
citool {
enable_option singlecommit
@@ -1058,6 +1064,7 @@ citool {
disable_option multicommit
disable_option branch
disable_option transport
+ disable_option picker
while {[llength $argv] > 0} {
set a [lindex $argv 0]
@@ -1080,6 +1087,9 @@ citool {
set argv [lrange $argv 1 end]
}
}
+pick {
+ disable_option gitdir_discovery
+}
}
######################################################################
@@ -1174,7 +1184,7 @@ proc unset_gitdir_vars {} {
# find repository.
set _gitdir {}
-if {$_gitdir eq {}} {
+if {[is_enabled gitdir_discovery]} {
if {[catch {
set _gitdir [git rev-parse --absolute-git-dir]
} err]} {
@@ -1186,7 +1196,7 @@ if {$_gitdir eq {}} {
}
set picked 0
-if {$_gitdir eq {}} {
+if {$_gitdir eq {} && [is_enabled picker]} {
unset_gitdir_vars
load_config 1
apply_config
@@ -1202,6 +1212,12 @@ if {$_gitdir eq {}} {
set picked 1
}
+if {$_gitdir eq {}} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Git directory not found:"] "\n\n$err"]
+ exit 1
+}
+
# find worktree, continue without if not required
if {[catch {
set _gitworktree [git rev-parse --show-toplevel]
@@ -3137,14 +3153,15 @@ blame {
return
}
citool -
-gui {
+gui -
+pick {
if {[llength $argv] != 0} {
usage
}
# 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 \
--
2.54.0.99.14
^ permalink raw reply related [flat|nested] 96+ messages in thread