gitk: encode arguments correctly with "open"

While "exec" uses a normal arguments list which is applied as command + arguments (and redirections, etc), "open" uses a single argument which is this command+arguments, where the command and arguments are a list inside this one argument to "open". Commit bb5cb23 (gitk: prevent overly long command lines 2023-05-08) changed several values from individual arguments in that list (hashes and file names), to a single value which is fed to git via redirection to its stdin using "open" [1]. However, it didn't ensure correctly that this aggregate value in this string is interpreted as a single element in this command+args list. It did just enough so that newlines (which is how these elements are concatenated) don't split this single list element. A followup commit at the same patchset: 7dd272e (gitk: escape file paths before piping to git log 2023-05-08) added a bit more, by escaping backslahes and spaces at the file names, so that at least it doesn't break when such file names get used there. But these are not enough. At the very least tab is missing, and more, and trying to manually escape every possible thing which can affect how this string is interpreted in a list is a sub-par approach. The solution is simply to tell tcl "this is a single list element". which we can do by aggregating this value completely normally (hashes and files separated by newlines), and then do [list $value]. So this is what this commit does, for all 3 places where bb5cb23 changed individual elements into an aggregate value. [1] That was not a fully accurate description. The accurate version is that this string originally included two lists: hashes and files. When used with "open" these lists correctly become the individual elements of these lists, even if they contain spaces etc, so the arguments which were used at this "git" commands were correct. Commit bb5cb23 couldn't use these two lists as-is, because it needed to process the individual elements in them (one element per line of the aggregate value), and the issue is that ensuring this aggregate is indeed interpreted as a single list element was sub-par. Note: all the (double) quotes before/after the modification are not required and with zero effect, even for \n. But this commit preserves the original quoting form intentionally. It can be cleaned up later. Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Avi Halachmi (:avih) committed Mar 7, 2025 at 13:48 UTC 8e3070aa5e331be45d4d03e3be41f84494fce129
1 file changed +3 -16
gitk
+3 -16
@@ -353,16 +353,6 @@ proc parseviewrevs {view revs} {
353 return $ret
354 }
355
356 -# Escapes a list of filter paths to be passed to git log via stdin. Note that
357 -# paths must not be quoted.
358 -proc escape_filter_paths {paths} {
359 - set escaped [list]
360 - foreach path $paths {
361 - lappend escaped [string map {\\ \\\\ "\ " "\\\ "} $path]
362 - }
363 - return $escaped
364 -}
365 -
356 # Start off a git log process and arrange to read its output
357 proc start_rev_list {view} {
358 global startmsecs commitidx viewcomplete curview
@@ -424,8 +414,7 @@ proc start_rev_list {view} {
414 if {[catch {
415 set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
416 --parents --boundary $args --stdin \
427 - "<<[join [concat $revs "--" \
428 - [escape_filter_paths $files]] "\\n"]"] r]
417 + [list "<<[join [concat $revs "--" $files] "\n"]"]] r]
418 } err]} {
419 error_popup "[mc "Error executing git log:"] $err"
420 return 0
@@ -578,9 +567,7 @@ proc updatecommits {} {
567 if {[catch {
568 set fd [open [concat | git log --no-color -z --pretty=raw $show_notes \
569 --parents --boundary $args --stdin \
581 - "<<[join [concat $revs "--" \
582 - [escape_filter_paths \
583 - $vfilelimit($view)]] "\\n"]"] r]
570 + [list "<<[join [concat $revs "--" $vfilelimit($view)] "\n"]"]] r]
571 } err]} {
572 error_popup "[mc "Error executing git log:"] $err"
573 return
@@ -10258,7 +10245,7 @@ proc getallcommits {} {
10245 if {$ids eq "--all"} {
10246 set cmd [concat $cmd "--all"]
10247 } else {
10261 - set cmd [concat $cmd --stdin "<<[join $ids "\\n"]"]
10248 + set cmd [concat $cmd --stdin [list "<<[join $ids "\n"]"]]
10249 }
10250 set fd [open $cmd r]
10251 fconfigure $fd -blocking 0