From: Mark Levedahl <mlevedahl@gmail•com>
To: git@vger•kernel.org
Cc: j6t@kdbg•org, egg_mushroomcow@foxmail•com, bootaina702@gmail•com,
Mark Levedahl <mlevedahl@gmail•com>
Subject: [PATCH v3 11/12] git-gui: check browser/blame arguments carefully
Date: Sun, 31 May 2026 19:02:24 -0400 [thread overview]
Message-ID: <20260531230225.126817-12-mlevedahl@gmail.com> (raw)
In-Reply-To: <20260531230225.126817-1-mlevedahl@gmail.com>
git gui offers two related commands, browser and blame, that provide
graphical interfaces driven by git ls-tree and git blame. As such, the
arguments to git-gui need to satisfy those two git commands. But,
git-gui does not assure this leading to confusing or incorrect results.
For instance 'git browser <non-existent path>' shows a blank browser
window rather than error message.
Also, commit 3e45ee1ef2 ("git-gui: Smarter command line parsing for
browser, blame", 2007-05-08) implemented code to allow giving path
before rev on the command line, and unconditionally uses the worktree to
disambiguate. As a result, the following command run in a current
git-gui checkout of the master branch shows the master branch version of
blame.tcl, when none should be shown as that file does not exist in
gitgui-0.6.0.
git gui blame lib/blame.tcl gitgui-0.6.0
This 'file before rev' feature in git-gui mirrors ideas considered when
git's user interface was very young, but no such feature is documented
for any git command. Rather than try to fix an idea git itself
rejected, let's just remove this broken and hopefully unused feature.
git-gui browser|blame both accept 'rev' and 'path' as command line
arguments. rev defaults to 'HEAD' if not given, while path must be
given. path names a directory tree to ls-tree or a file to blame. path
must exist in rev for ls-tree and for blame. In addition git blame will
include uncommitted changes from the worktree file at 'path' if rev is
not given (thus defaulting to HEAD), but still requires that the file
exists in HEAD.
So, let's clean up the parser to check that the arguments are usable.
- give a full synopsis, including '--' that may be used to separate rev and
path. (as path is the required final arg, -- gives no extra info)
- explicitly check the number of arguments
- use rev-parse to assure a user supplied rev is valid
- use ls-tree to assure that path exists in rev
- for blame only, with no rev given and a worktree existing, also assure
that path points to a file in the worktree
With these changes, error messages are thrown by the parser if the path
or rev are not known: no blank or erroneous displays are created. Also,
this avoids accessing the worktree except in the specific use case
supported by blame / git-blame, meaning browser|blame now also work
without a worktree.
Signed-off-by: Mark Levedahl <mlevedahl@gmail•com>
---
git-gui.sh | 122 +++++++++++++++++++++++++++--------------------------
1 file changed, 63 insertions(+), 59 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index 16d6b3051a..22939215a6 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3000,101 +3000,105 @@ proc normalize_relpath {path} {
}
}
+proc show_parse_err {err} {
+ if {[tk windowingsystem] eq "win32"} {
+ catch {wm withdraw .}
+ error_popup $err
+ } else {
+ puts stderr $err
+ }
+ exit 1
+}
+
# -- 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>}
+ set required_pathtype blob
} else {
- set subcommand_args {rev? path}
+ set subcommand_args {[rev] [--] <dirname>}
+ set required_pathtype tree
}
- if {$argv eq {}} usage
+ set maxargs [llength $subcommand_args]
+ set nargs [llength $argv]
+ if {$nargs < 1 || $nargs > $maxargs} usage
set head {}
set path {}
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
+ set iarg 0
+ foreach a $argv {
+ incr iarg
+ if {$iarg == $nargs} {
+ # final argument is path
+ set path [normalize_relpath [file join $_prefix $a]]
} elseif {$a eq {--}} {
- if {$path ne {}} {
- if {$head ne {}} usage
- set head $path
- set path {}
+ # allow before required final arg that must be path
+ if {$iarg != $nargs - 1} {
+ usage
}
- set is_path 1
} elseif {[regexp {^--line=(\d+)$} $a a lnum]} {
- if {$jump_spec ne {} || $head ne {}} usage
+ # --line can only be the first arg
+ if {$iarg != 1 || $subcommand ne {blame}} 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 {}
+ # If head not given, use current branch (HEAD),
+ # and blame will use worktree if there is one.
+ set use_worktree 0
+ if {$head eq {}} {
+ load_current_branch
+ set head $current_branch
+ if {$subcommand eq {blame} && ![is_bare]} {
+ if {![file isfile $path]} {
+ show_parse_err [mc "fatal: no such file '%s' in worktree" $path]
+ }
+ set use_worktree 1
+ }
+ } else {
+ if {[catch {
+ set commitid \
+ [git rev-parse --verify --end-of-options \
+ [strcat $head "^{commit}"]]
+ }]} {
+ show_parse_err [mc "fatal: '%s' is not a valid rev'" $head]
} else {
- set path [normalize_relpath $_prefix$head]
- set head {}
+ set 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
- }
+ # check path is known in head, and is file / directory as required
+ set pathtype {}
+ catch {set pathtype [git ls-tree {--format=%(objecttype)} $head $path]}
+ if {$pathtype ne {} && $path eq {.}} {
+ # ls-tree gives contents of root-dir, we need root-dir itself
+ set pathtype {tree}
+ }
+
+ if {$pathtype ne $required_pathtype} {
+ switch -- $required_pathtype {
+ tree {show_parse_err \
+ [mc "'%s' is not a directory in rev '%s'" $path $head]}
+ blob {show_parse_err \
+ [mc "'%s' is not a filename in rev '%s'" $path $head]}
}
- set current_branch $head
}
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::new $head $path $jump_spec
+ blame::new [expr {$use_worktree ? {} : $head}] $path $jump_spec
}
}
return
--
2.54.0.99.14
next prev parent reply other threads:[~2026-05-31 23:03 UTC|newest]
Thread overview: 134+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 16:28 [PATCH] git-gui: handle bare repo or missing worktree Shroom Moo
2026-04-29 6:58 ` Johannes Sixt
2026-04-29 17:32 ` [PATCH v2 1/1] git-gui: protect rev-parse --show-toplevel call Shroom Moo
2026-04-29 20:14 ` Mark Levedahl
2026-04-30 10:02 ` [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir Shroom Moo
2026-04-30 16:18 ` Mark Levedahl
2026-05-01 10:22 ` [PATCH v3 1/1] git-gui: handle missing worktree and separated Shroom Moo
2026-05-01 13:13 ` [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir Johannes Sixt
2026-05-01 16:42 ` Mark Levedahl
2026-05-02 21:51 ` Mark Levedahl
2026-05-03 8:53 ` Johannes Sixt
2026-05-04 15:13 ` Mark Levedahl
2026-05-05 3:40 ` Mark Levedahl
2026-05-06 7:32 ` Johannes Sixt
2026-05-06 11:27 ` Mark Levedahl
2026-05-06 12:57 ` Johannes Sixt
2026-05-06 14:05 ` Mark Levedahl
2026-05-07 5:09 ` Mark Levedahl
2026-05-01 10:54 ` [PATCH v4 " Shroom Moo
2026-05-04 14:59 ` [PATCH v5 1/1] git-gui: restructure repository startup Shroom Moo
2026-05-06 7:15 ` Johannes Sixt
2026-05-06 20:27 ` [PATCH v6 0/3] git-gui: robustify startup and fix environment handling Shroom Moo
2026-05-09 13:37 ` [PATCH v7 " Shroom Moo
2026-05-14 14:28 ` Mark Levedahl
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-15 15:54 ` Johannes Sixt
2026-05-16 13:38 ` Mark Levedahl
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
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
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
2026-05-16 14:29 ` Mark Levedahl
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
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 ` [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
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
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
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
2026-05-19 8:16 ` Johannes Sixt
2026-05-19 19:00 ` Mark Levedahl
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
2026-05-19 8:21 ` Johannes Sixt
2026-05-19 18:45 ` Mark Levedahl
2026-05-19 21:15 ` 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
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
2026-05-22 11:54 ` Mark Levedahl
2026-05-23 8:18 ` Johannes Sixt
2026-05-23 11:46 ` Aina Boot
2026-05-23 16:08 ` Mark Levedahl
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
2026-05-20 20:24 ` [PATCH v2 03/11] git-gui: use --absolute-git-dir 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
2026-05-22 8:46 ` Johannes Sixt
2026-05-22 12:04 ` Mark Levedahl
2026-05-22 23:00 ` Mark Levedahl
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 ` [PATCH v2 06/11] git-gui: use git rev-parse for worktree discovery Mark Levedahl
2026-05-23 13:26 ` Johannes Sixt
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
2026-05-22 15:09 ` Shroom Moo
2026-05-22 16:57 ` Mark Levedahl
2026-05-23 8:01 ` Johannes Sixt
2026-05-23 11:47 ` Shroom Moo
2026-05-23 14:06 ` Johannes Sixt
2026-05-23 15:33 ` Mark Levedahl
2026-05-20 20:24 ` [PATCH v2 08/11] git-gui: use HEAD as current branch when detached (bug fix) Mark Levedahl
2026-05-23 14:08 ` Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 09/11] git-gui: allow specifying path '.' to the browser Mark Levedahl
2026-05-23 14:23 ` Johannes Sixt
2026-05-23 15:43 ` Mark Levedahl
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
2026-05-22 15:05 ` Shroom Moo
2026-05-23 18:19 ` [PATCH] fixup git-gui: allow blame to show uncommitted changes Mark Levedahl
2026-05-23 20:31 ` [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation Johannes Sixt
2026-05-20 20:24 ` [PATCH v2 11/11] git-gui: add gui and pick as explicit subcommands Mark Levedahl
2026-05-24 7:00 ` Johannes Sixt
2026-05-24 7:16 ` [PATCH v2 00/11] Improve git gui operation without a worktree Johannes Sixt
2026-05-25 16:02 ` Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 00/12] " Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 01/12] git-gui: use HEAD as current branch when detached Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 02/12] git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 03/12] git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 04/12] git-gui: do not change global vars in choose_repository::pick Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 05/12] git-gui: use --absolute-git-dir Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 06/12] git-gui: use rev-parse exclusively to find a repository Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 07/12] git-gui: use git rev-parse for worktree discovery Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 08/12] git-gui: simplify [is_bare] to report if a worktree is known Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 09/12] git-gui: try harder to find worktree from gitdir Mark Levedahl
2026-05-31 23:02 ` [PATCH v3 10/12] git-gui: allow specifying path '.' to the browser Mark Levedahl
2026-05-31 23:02 ` Mark Levedahl [this message]
2026-05-31 23:02 ` [PATCH v3 12/12] git-gui: add gui and pick as explicit subcommands Mark Levedahl
2026-06-02 17:34 ` [PATCH v3 00/12] Improve git gui operation without a worktree Johannes Sixt
2026-06-02 18:54 ` Mark Levedahl
2026-06-02 21:05 ` Johannes Sixt
[not found] ` <20260509133756.1367-1-egg_mushroomcow@foxmail.com>
2026-05-09 13:37 ` [PATCH v7 1/3] git-gui: restructure repository startup Shroom Moo
2026-05-15 8:26 ` Johannes Sixt
2026-05-09 13:37 ` [PATCH v7 2/3] git-gui: disable gitk visualization when no worktree available Shroom Moo
2026-05-15 8:28 ` Johannes Sixt
2026-05-09 13:37 ` [PATCH v7 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early Shroom Moo
2026-05-15 8:28 ` Johannes Sixt
[not found] ` <20260506202751.3294-1-egg_mushroomcow@foxmail.com>
2026-05-06 20:27 ` [PATCH v6 1/3] git-gui: restructure repository startup Shroom Moo
2026-05-06 20:27 ` [PATCH v6 2/3] git-gui: disable gitk visualization when no worktree available Shroom Moo
2026-05-06 20:27 ` [PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early Shroom Moo
2026-05-07 15:50 ` Mark Levedahl
2026-05-09 8:46 ` Aina Boot
2026-05-09 9:55 ` Shroom Moo
2026-04-29 18:28 ` [PATCH] git-gui: handle bare repo or missing worktree Shroom Moo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260531230225.126817-12-mlevedahl@gmail.com \
--to=mlevedahl@gmail$(echo .)com \
--cc=bootaina702@gmail$(echo .)com \
--cc=egg_mushroomcow@foxmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=j6t@kdbg$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox