gitk support for git log -L
cover gitk support for git log -L
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
The hard part is that AFAICS this is the first history display accessible through gitk that can only be computed in one go. The existing displays all work by having git-log perform a preliminary search for the involved commits (or in some cases, only part of the range while we fetch more). log -L has to compute all the diffs anyway, so nothing can be saved by attempting this; it is better to load everything in bulk from a single git-log invocation.
Thus, patches 1--3 implement the infrastructure required to be able to work from a single git-log command.
I would have loved to instead make a feature that also generalizes to
git log --parents <whatever> | gitk --read-stdin
(or some other similar option). This would make for much easier testing of new git-log options. Unfortunately this seems much harder to achieve in the current structure of gitk.
Note: all my Tk-ing is computationally indistinguishable from cargo culting. Please review with a grain of salt.
Thomas Rast (4):
gitk: refactor per-line part of getblobdiffline and its support
gitk: split out diff part in $commitinfo
gitk: support showing the gathered inline diffs
gitk: recognize -L option
gitk | 462 ++++++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 266 insertions(+), 196 deletions(-)
Signature · 1 line
1.8.3.496.g0d0267b
PATCH 1/4 gitk: refactor per-line part of getblobdiffline and its support
For later use with data sources other than a pipe, refactor the big worker part of getblobdiffline to a separate function parseblobdiffline. Also refactor its initialization and wrap-up to separate routines.
--- gitk | 408 +++++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 213 insertions(+), 195 deletions(-)
gitk +213 -195
@@ -7710,15 +7710,25 @@ proc changeworddiff {name ix op} { reselectline } +proc initblobdiffvars {} { + global diffencoding targetline diffnparents + global diffinhdr currdiffsubmod diffseehere + set targetline {} + set diffnparents 0 + set diffinhdr 0 + set diffencoding [get_path_encoding {}] + set currdiffsubmod "" + set diffseehere -1 +} + proc getblobdiffs {ids} { global blobdifffd diffids env - global diffinhdr treediffs + global treediffs global diffcontext global ignorespace global worddiff global limitdiffs vfilelimit curview - global diffencoding targetline diffnparents - global git_version currdiffsubmod + global git_version set textconv {} if {[package vcompare $git_version "1.6.1"] >= 0} {
@@ -7742,13 +7752,9 @@ proc getblobdiffs {ids} { error_popup [mc "Error getting diffs: %s" $err] return } - set targetline {} - set diffnparents 0 - set diffinhdr 0 - set diffencoding [get_path_encoding {}] fconfigure $bdf -blocking 0 -encoding binary -eofchar {} set blobdifffd($ids) $bdf - set currdiffsubmod "" + initblobdiffvars filerun $bdf [list getblobdiffline $bdf $diffids] }
@@ -7814,13 +7820,17 @@ proc makediffhdr {fname ids} { set diffline 0 } +proc blobdiffmaybeseehere {ateof} { + global diffseehere + if {$diffseehere >= 0} { + mark_ctext_line [lindex [split $diffseehere .] 0] + } + maybe_scroll_ctext ateof +} + proc getblobdiffline {bdf ids} { - global diffids blobdifffd ctext curdiffstart - global diffnexthead diffnextnote difffilestart - global ctext_file_names ctext_file_lines - global diffinhdr treediffs mergemax diffnparents - global diffencoding jump_to_here targetline diffline currdiffsubmod - global worddiff + global diffids blobdifffd + global ctext set nr 0 $ctext conf -state normal
@@ -7829,212 +7839,220 @@ proc getblobdiffline {bdf ids} { catch {close $bdf} return 0 } - if {![string compare -length 5 "diff " $line]} { - if {![regexp {^diff (--cc|--git) } $line m type]} { - set line [encoding convertfrom $line] - $ctext insert end "$line\n" hunksep - continue + parseblobdiffline $ids $line + } + $ctext conf -state disabled + blobdiffmaybeseehere [eof $bdf] + if {[eof $bdf]} { + catch {close $bdf} + return 0 + } + return [expr {$nr >= 1000? 2: 1}] +} + +proc parseblobdiffline {ids line} { + global ctext curdiffstart + global diffnexthead diffnextnote difffilestart + global ctext_file_names ctext_file_lines + global diffinhdr treediffs mergemax diffnparents + global diffencoding jump_to_here targetline diffline currdiffsubmod + global worddiff diffseehere + + if {![string compare -length 5 "diff " $line]} { + if {![regexp {^diff (--cc|--git) } $line m type]} { + set line [encoding convertfrom $line] + $ctext insert end "$line\n" hunksep + continue + } + # start of a new file + set diffinhdr 1 + $ctext insert end "\n" + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names "" + lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + $ctext insert end "\n" filesep + + if {$type eq "--cc"} { + # start of a new file in a merge diff + set fname [string range $line 10 end] + if {[lsearch -exact $treediffs($ids) $fname] < 0} { + lappend treediffs($ids) $fname + add_flist [list $fname] } - # start of a new file - set diffinhdr 1 - $ctext insert end "\n" - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names "" - lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] - $ctext insert end "\n" filesep - - if {$type eq "--cc"} { - # start of a new file in a merge diff - set fname [string range $line 10 end] - if {[lsearch -exact $treediffs($ids) $fname] < 0} { - lappend treediffs($ids) $fname - add_flist [list $fname] - } + } else { + set line [string range $line 11 end] + # If the name hasn't changed the length will be odd, + # the middle char will be a space, and the two bits either + # side will be a/name and b/name, or "a/name" and "b/name". + # If the name has changed we'll get "rename from" and + # "rename to" or "copy from" and "copy to" lines following + # this, and we'll use them to get the filenames. + # This complexity is necessary because spaces in the + # filename(s) don't get escaped. + set l [string length $line] + set i [expr {$l / 2}] + if {!(($l & 1) && [string index $line $i] eq " " && + [string range $line 2 [expr {$i - 1}]] eq \ + [string range $line [expr {$i + 3}] end])} { + return + } + # unescape if quoted and chop off the a/ from the front + if {[string index $line 0] eq "\""} { + set fname [string range [lindex $line 0] 2 end] } else { - set line [string range $line 11 end] - # If the name hasn't changed the length will be odd, - # the middle char will be a space, and the two bits either - # side will be a/name and b/name, or "a/name" and "b/name". - # If the name has changed we'll get "rename from" and - # "rename to" or "copy from" and "copy to" lines following - # this, and we'll use them to get the filenames. - # This complexity is necessary because spaces in the - # filename(s) don't get escaped. - set l [string length $line] - set i [expr {$l / 2}] - if {!(($l & 1) && [string index $line $i] eq " " && - [string range $line 2 [expr {$i - 1}]] eq \ - [string range $line [expr {$i + 3}] end])} { - continue - } - # unescape if quoted and chop off the a/ from the front - if {[string index $line 0] eq "\""} { - set fname [string range [lindex $line 0] 2 end] - } else { - set fname [string range $line 2 [expr {$i - 1}]] - } + set fname [string range $line 2 [expr {$i - 1}]] } - makediffhdr $fname $ids + } + makediffhdr $fname $ids - } elseif {![string compare -length 16 "* Unmerged path " $line]} { - set fname [encoding convertfrom [string range $line 16 end]] - $ctext insert end "\n" - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names $fname - lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + } elseif {![string compare -length 16 "* Unmerged path " $line]} { + set fname [encoding convertfrom [string range $line 16 end]] + $ctext insert end "\n" + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names $fname + lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + $ctext insert end "$line\n" filesep + set i [lsearch -exact $treediffs($ids) $fname] + if {$i >= 0} { + setinlist difffilestart $i $curdiffstart + } + + } elseif {![string compare -length 2 "@@" $line]} { + regexp {^@@+} $line ats + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" hunksep + if {[regexp { \+(\d+),\d+ @@} $line m nl]} { + set diffline $nl + } + set diffnparents [expr {[string length $ats] - 1}] + set diffinhdr 0 + + } elseif {![string compare -length 10 "Submodule " $line]} { + # start of a new submodule + if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { + set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] + } else { + set fname [string range $line 10 [expr [string first "contains " $line] - 2]] + } + if {$currdiffsubmod != $fname} { + $ctext insert end "\n"; # Add newline after commit message + } + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names "" + if {$currdiffsubmod != $fname} { + lappend ctext_file_lines $fname + makediffhdr $fname $ids + set currdiffsubmod $fname + $ctext insert end "\n$line\n" filesep + } else { $ctext insert end "$line\n" filesep + } + } elseif {![string compare -length 3 " >" $line]} { + set $currdiffsubmod "" + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + } elseif {![string compare -length 3 " <" $line]} { + set $currdiffsubmod "" + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 + } elseif {$diffinhdr} { + if {![string compare -length 12 "rename from " $line]} { + set fname [string range $line [expr 6 + [string first " from " $line] ] end] + if {[string index $fname 0] eq "\""} { + set fname [lindex $fname 0] + } + set fname [encoding convertfrom $fname] set i [lsearch -exact $treediffs($ids) $fname] if {$i >= 0} { setinlist difffilestart $i $curdiffstart } - - } elseif {![string compare -length 2 "@@" $line]} { - regexp {^@@+} $line ats - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" hunksep - if {[regexp { \+(\d+),\d+ @@} $line m nl]} { - set diffline $nl + } elseif {![string compare -length 10 $line "rename to "] || + ![string compare -length 8 $line "copy to "]} { + set fname [string range $line [expr 4 + [string first " to " $line] ] end] + if {[string index $fname 0] eq "\""} { + set fname [lindex $fname 0] } - set diffnparents [expr {[string length $ats] - 1}] + makediffhdr $fname $ids + } elseif {[string compare -length 3 $line "---"] == 0} { + # do nothing + return + } elseif {[string compare -length 3 $line "+++"] == 0} { set diffinhdr 0 + return + } + $ctext insert end "$line\n" filesep - } elseif {![string compare -length 10 "Submodule " $line]} { - # start of a new submodule - if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { - set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] - } else { - set fname [string range $line 10 [expr [string first "contains " $line] - 2]] - } - if {$currdiffsubmod != $fname} { - $ctext insert end "\n"; # Add newline after commit message - } - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names "" - if {$currdiffsubmod != $fname} { - lappend ctext_file_lines $fname - makediffhdr $fname $ids - set currdiffsubmod $fname - $ctext insert end "\n$line\n" filesep - } else { - $ctext insert end "$line\n" filesep - } - } elseif {![string compare -length 3 " >" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 - } elseif {$diffinhdr} { - if {![string compare -length 12 "rename from " $line]} { - set fname [string range $line [expr 6 + [string first " from " $line] ] end] - if {[string index $fname 0] eq "\""} { - set fname [lindex $fname 0] - } - set fname [encoding convertfrom $fname] - set i [lsearch -exact $treediffs($ids) $fname] - if {$i >= 0} { - setinlist difffilestart $i $curdiffstart + } else { + set line [string map {\x1A ^Z} \ + [encoding convertfrom $diffencoding $line]] + # parse the prefix - one ' ', '-' or '+' for each parent + set prefix [string range $line 0 [expr {$diffnparents - 1}]] + set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] + set words_pre_markup "" + set words_post_markup "" + if {[string trim $prefix " -+"] eq {}} { + # prefix only has " ", "-" and "+" in it: normal diff line + set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } + if {$num >= 0} { + # removed line, first parent with line is $num + if {$num >= $mergemax} { + set num "max" } - } elseif {![string compare -length 10 $line "rename to "] || - ![string compare -length 8 $line "copy to "]} { - set fname [string range $line [expr 4 + [string first " to " $line] ] end] - if {[string index $fname 0] eq "\""} { - set fname [lindex $fname 0] + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "\[-$line-\]" $tag$num + } else { + $ctext insert end "$line" $tag$num } - makediffhdr $fname $ids - } elseif {[string compare -length 3 $line "---"] == 0} { - # do nothing - continue - } elseif {[string compare -length 3 $line "+++"] == 0} { - set diffinhdr 0 - continue - } - $ctext insert end "$line\n" filesep - - } else { - set line [string map {\x1A ^Z} \ - [encoding convertfrom $diffencoding $line]] - # parse the prefix - one ' ', '-' or '+' for each parent - set prefix [string range $line 0 [expr {$diffnparents - 1}]] - set tag [expr {$diffnparents > 1? "m": "d"}] - set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] - set words_pre_markup "" - set words_post_markup "" - if {[string trim $prefix " -+"] eq {}} { - # prefix only has " ", "-" and "+" in it: normal diff line - set num [string first "-" $prefix] - if {$dowords} { - set line [string range $line 1 end] + if {!$dowords} { + $ctext insert end "\n" $tag$num } - if {$num >= 0} { - # removed line, first parent with line is $num - if {$num >= $mergemax} { - set num "max" - } - if {$dowords && $worddiff eq [mc "Markup words"]} { - $ctext insert end "\[-$line-\]" $tag$num - } else { - $ctext insert end "$line" $tag$num - } - if {!$dowords} { - $ctext insert end "\n" $tag$num - } - } else { - set tags {} - if {[string first "+" $prefix] >= 0} { - # added line - lappend tags ${tag}result - if {$diffnparents > 1} { - set num [string first " " $prefix] - if {$num >= 0} { - if {$num >= $mergemax} { - set num "max" - } - lappend tags m$num + } else { + set tags {} + if {[string first "+" $prefix] >= 0} { + # added line + lappend tags ${tag}result + if {$diffnparents > 1} { + set num [string first " " $prefix] + if {$num >= 0} { + if {$num >= $mergemax} { + set num "max" } + lappend tags m$num } - set words_pre_markup "{+" - set words_post_markup "+}" } - if {$targetline ne {}} { - if {$diffline == $targetline} { - set seehere [$ctext index "end - 1 chars"] - set targetline {} - } else { - incr diffline - } - } - if {$dowords && $worddiff eq [mc "Markup words"]} { - $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + set words_pre_markup "{+" + set words_post_markup "+}" + } + if {$targetline ne {}} { + if {$diffline == $targetline} { + set diffseehere [$ctext index "end - 1 chars"] + set targetline {} } else { - $ctext insert end "$line" $tags - } - if {!$dowords} { - $ctext insert end "\n" $tags + incr diffline } } - } elseif {$dowords && $prefix eq "~"} { - $ctext insert end "\n" {} - } else { - # "\ No newline at end of file", - # or something else we don't recognize - $ctext insert end "$line\n" hunksep + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + } else { + $ctext insert end "$line" $tags + } + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} + } else { + # "\ No newline at end of file", + # or something else we don't recognize + $ctext insert end "$line\n" hunksep } } - if {[info exists seehere]} { - mark_ctext_line [lindex [split $seehere .] 0] - } - maybe_scroll_ctext [eof $bdf] - $ctext conf -state disabled - if {[eof $bdf]} { - catch {close $bdf} - return 0 - } - return [expr {$nr >= 1000? 2: 1}] } proc changediffdisp {} {
Signature · 1 line
1.8.3.496.g0d0267b
PATCH 2/4 gitk: split out diff part in $commitinfo
In reply to Thomas RastSo far we just parsed everything after the headers into the "comment" bit of $commitinfo, including notes and -- if you gave weird options -- the diff.
Split out the diff, if any, into a separate field. It's easy to recognize, since the log message is indented but the /^diff / that starts a diff is not.
--- gitk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
gitk +7 -1
@@ -1704,8 +1704,14 @@ proc parsecommit {id contents listed} { set comment $newcomment } set hasnote [string first "\nNotes:\n" $contents] + set diff "" + set i [string first "\ndiff" $comment] + if {$i >= 0} { + set diff [string range $comment $i end] + set comment [string range $comment 0 $i] + } set commitinfo($id) [list $headline $auname $audate \ - $comname $comdate $comment $hasnote] + $comname $comdate $comment $hasnote $diff] } proc getcommit {id} {
Signature · 1 line
1.8.3.496.g0d0267b
PATCH 3/4 gitk: support showing the gathered inline diffs
In reply to Thomas RastThe previous commit split the diffs into a separate field. Now we actually want to show them.
To that end we use the stored diff, and
- process it once to build a fake "tree diff", i.e., a list of all changed files;
- feed it through parseblobdiffline to actually format it into the $ctext field, like the existing diff machinery would.
--- gitk | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
gitk +38 -0
@@ -156,10 +156,12 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global vinlinediff global worddiff git_version set vdatemode($n) 0 set vmergeonly($n) 0 + set vinlinediff($n) 0 set glflags {} set diffargs {} set nextisval 0
@@ -7086,6 +7088,7 @@ proc selectline {l isnew {desired_loc {}}} { global cmitmode showneartags allcommits global targetrow targetid lastscrollrows global autoselect autosellen jump_to_here + global vinlinediff catch {unset pending_select} $canv delete hover
@@ -7227,6 +7230,8 @@ proc selectline {l isnew {desired_loc {}}} { init_flist [mc "Comments"] if {$cmitmode eq "tree"} { gettree $id + } elseif {$vinlinediff($curview) == 1} { + showinlinediff $id } elseif {[llength $olds] <= 1} { startdiff $id } else {
@@ -7563,6 +7568,39 @@ proc startdiff {ids} { } } +proc showinlinediff {ids} { + global commitinfo commitdata ctext + global treediffs + + set info $commitinfo($ids) + set diff [lindex $info 7] + set difflines [split $diff "\n"] + + initblobdiffvars + set treediff {} + + set inhdr 0 + foreach line $difflines { + if {![string compare -length 5 "diff " $line]} { + set inhdr 1 + } elseif {$inhdr && ![string compare -length 4 "+++ " $line]} { + # offset also accounts for the b/ prefix + lappend treediff [string range $line 6 end] + set inhdr 0 + } + } + + set treediffs($ids) $treediff + add_flist $treediff + + $ctext conf -state normal + foreach line $difflines { + parseblobdiffline $ids $line + } + maybe_scroll_ctext 1 + $ctext conf -state disabled +} + # If the filename (name) is under any of the passed filter paths # then return true to include the file in the listing. proc path_filter {filter name} {
Signature · 1 line
1.8.3.496.g0d0267b
PATCH 4/4 gitk: recognize -L option
In reply to Thomas RastThis gives line-log support to gitk, by exploiting the new support for processing and showing "inline" diffs straight from the git-log output.
Note that we 'set allknown 0', which is a bit counterintuitive since this is a "known" option. But that flag prevents gitk from thinking it can optimize the view by running rev-list to see the topology; in the -L case that doesn't work.
--- gitk | 8 ++++++++ 1 file changed, 8 insertions(+)
gitk +8 -0
@@ -235,6 +235,14 @@ proc parseviewargs {n arglist} { set filtered 1 lappend glflags $arg } + "-L*" { + # Line-diff: filtered, and diffs must be read as part + # of the log output + set filtered 1 + set vinlinediff($n) 1 + set allknown 0 + lappend glflags $arg + } "-n" { # This appears to be the only one that has a value as a # separate word following it
Signature · 1 line
1.8.3.496.g0d0267b
Quoted 2 lines · Thomas Rast
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'?
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chQuoted 7 lines · Thomas Rast
Quoted 2 lines · Thomas Rast
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'?
Jens pointed out privately that the handling of unstuck -L options is unfortunate, to put it mildly. I'll send a reroll.
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chAm 29.07.2013 21:37, schrieb Thomas Rast:
Quoted 4 lines
Quoted 2 lines · Thomas Rast
Thomas Rast <[email protected]> writes:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'?
Jens pointed out privately that the handling of unstuck -L options is unfortunate, to put it mildly. I'll send a reroll.
But as soon as that is fixed I'd really like to see this applied, as I think gitk is the perfect tool to show history information.
Quoted 8 lines · Jens Lehmann
Am 29.07.2013 21:37, schrieb Thomas Rast:
Quoted 7 lines
Quoted 5 lines · Thomas Rast
Quoted 2 lines · Thomas Rast
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'?
Jens pointed out privately that the handling of unstuck -L options is unfortunate, to put it mildly. I'll send a reroll.
But as soon as that is fixed I'd really like to see this applied, as I think gitk is the perfect tool to show history information.
Unfortunately it's turning out to be harder than I hoped. gitk runs the arguments through git-rev-parse, which only knows that -n gets an unstuck argument. Consequently, gitk accepts an unstuck -n but only stuck forms of -S and -G.
Fixing it through git-rev-parse feels wrong; rev-parse is supposed to know about rev-list options, but -S and -G only make sense in diff-generating walks, and -L only makes any sense at all for git-log.
I'm tempted to leave it at the existing patches for now. That does mean that -L can only be used in the stuck form; it's the same for -S and -G already. Then in a later series we can change gitk's argument parsing to properly treat the options directly, passing only the remaining arguments through to rev-parse to use the usual revision/filename distinction logic.
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chHi Thomas,
Quoted 10 lines · Thomas Rast
Quoted 8 lines · Jens Lehmann
Am 29.07.2013 21:37, schrieb Thomas Rast:
Quoted 7 lines
Quoted 5 lines · Thomas Rast
Quoted 2 lines · Thomas Rast
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Paul, any news on this? Any chance we can get it into the next release, since that will also be the first release to ship with 'git log -L'?
Jens pointed out privately that the handling of unstuck -L options is unfortunate, to put it mildly. I'll send a reroll.
But as soon as that is fixed I'd really like to see this applied, as I think gitk is the perfect tool to show history information.
One thing I worry about is having gitk storing in memory not just the history graph but also all the diffs (assuming I have understood correctly what you're doing). Gitk's memory consumption is already pretty large. However, I can't see an alternative at this point.
Quoted 4 lines
Unfortunately it's turning out to be harder than I hoped. gitk runs the arguments through git-rev-parse, which only knows that -n gets an unstuck argument. Consequently, gitk accepts an unstuck -n but only stuck forms of -S and -G.
Excuse my ignorance, but what do you mean by "stuck" vs. "unstuck"?
Thanks,
Paul.
Quoted 12 lines · Paul Mackerras
Hi Thomas,
Quoted 9 lines · Thomas Rast
Quoted 7 lines · Jens Lehmann
Am 29.07.2013 21:37, schrieb Thomas Rast:
Quoted 6 lines
Quoted 4 lines · Thomas Rast
Quoted 2 lines · Thomas Rast
Now that git log -L has hit master, I figure it's time to discuss the corresponding change to gitk.
[...]
Quoted 5 lines
One thing I worry about is having gitk storing in memory not just the history graph but also all the diffs (assuming I have understood correctly what you're doing). Gitk's memory consumption is already pretty large. However, I can't see an alternative at this point.
I don't think there is one. log -L is pretty much an "all or nothing" thing at this point. I suppose if we really found that the diffs are regularly too big to be manageable for gitk, we could invent a porcelain mode where 'log -L' just prints the detected commits and corresponding line ranges, and then have a new option to diff-tree to let it again filter that range.
But note that ordinary 'git log -L' also buffers the entire set of diffs within less. The memory consumption of gitk to hold the same diffs in memory should be only a small factor of what less uses in the same scenario. Furthermore, users will typically ask for a small region of code (one function, or some such), so the diffs themselves are usually quite small, nowhere near the size of the full commit diffs.
Quoted 6 lines
Quoted 4 lines
Unfortunately it's turning out to be harder than I hoped. gitk runs the arguments through git-rev-parse, which only knows that -n gets an unstuck argument. Consequently, gitk accepts an unstuck -n but only stuck forms of -S and -G.
Excuse my ignorance, but what do you mean by "stuck" vs. "unstuck"?
Whether the option value is a separate argument in argv, or directly stuck to the option.
stuck: gitk -L:foo:main.c unstuck: gitk -L :foo:main.c
Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chQuoted 7 lines · Thomas Rast
Whether the option value is a separate argument in argv, or directly stuck to the option.
stuck: gitk -L:foo:main.c unstuck: gitk -L :foo:main.cExisting gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.
I somehow thought that we encourage the "stuck/sticked" form, to reduce things the users need to remember to cope better with options with optional value.
Quoted 5 lines · Junio C Hamano
Quoted 3 lines · Thomas Rast
Whether the option value is a separate argument in argv, or directly stuck to the option.
Quoted 8 lines
Quoted 4 lines
stuck: gitk -L:foo:main.c unstuck: gitk -L :foo:main.cExisting gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.
I somehow thought that we encourage the "stuck/sticked" form, to reduce things the users need to remember to cope better with options with optional value.
I just looked into this again, to get it rolling.
Am I reading you correctly as saying that any support for the unstuck form is entirely coincidental, and it's okay to support only the stuck version in new gitk?
Note that the support for 'git log -L' supports both, and it irks me that the user can't switch back and forth between using 'gitk' and 'git log' while leaving the rest of the command intact.
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chQuoted 9 lines · Thomas Rast
Quoted 7 lines · Junio C Hamano
Quoted 1 line · Thomas Rast
Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.
I somehow thought that we encourage the "stuck/sticked" form, to reduce things the users need to remember to cope better with options with optional value.
Quoted 5 lines
I just looked into this again, to get it rolling.
Am I reading you correctly as saying that any support for the unstuck form is entirely coincidental, and it's okay to support only the stuck version in new gitk?
Sort of. :)
gitcli(7) says that the sticked form is to be preferred "when you are scripting git". But most git commands use parse-options, which of course supports both forms and makes life easier for humans.
Support for just the sticked form is better than nothing, especially if the gitk(1) manpage gains a note about it. In the long run I guess the ideal would be to add a parse-options-like library to the tcl support.
My two cents,
Jonathan
PATCH 1/1 Documentation: revamp gitk(1)
The gitk manpage suffers from a bit of neglect: there have been only minor changes, and no changes to the set of options documented, since a2df1fb (Documentation: New GUI configuration and command-line options., 2008-11-13). In the meantime, the set of rev-list options has been expanded several times by options that are useful in gitk, e.g., --ancestry-path and the optional globbing for --branches, --tags and --remotes.
Restructure and expand the manpage. List more options that the author perceives as useful, while remaining somewhat terse. Ideally the user should not have to look up any of the references, but we dispense with precise explanations in some places and refer to git-log(1) instead.
Note that the options that have an easy GUI equivalent (e.g., --word-diff, -S, --grep) are deliberately not listed even in the cases where they simply fill in the GUI fields.
---
Quoted 4 lines · Jonathan Nieder
Support for just the sticked form is better than nothing, especially if the gitk(1) manpage gains a note about it. In the long run I guess the ideal would be to add a parse-options-like library to the tcl support.
Ok. I'm generally not happy with the state of that manpage, so I took the chance to improve it (and include a note about sticked forms). The approach is really my own opinion; I ran a half-hearted attempt at an IRC survey but none of the willing victims had any 'gitk' invocations in their history.
I'll hold the gitk patches until we get this one sorted out, but then just do the sticked form as before.
Documentation/gitk.txt | 107 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 83 insertions(+), 24 deletions(-)
Documentation/gitk.txt +83 -24
@@ -8,7 +8,7 @@ gitk - The Git repository browser SYNOPSIS -------- [verse] -'gitk' [<option>...] [<revs>] [--] [<path>...] +'gitk' [<options>] [<revision range>] [\--] [<path>...] DESCRIPTION -----------
@@ -16,21 +16,38 @@ Displays changes in a repository or a selected set of commits. This includes visualizing the commit graph, showing information related to each commit, and the files in the trees of each revision. -Historically, gitk was the first repository browser. It's written in tcl/tk -and started off in a separate repository but was later merged into the main -Git repository. - OPTIONS ------- -To control which revisions to show, the command takes options applicable to -the 'git rev-list' command (see linkgit:git-rev-list[1]). -This manual page describes only the most -frequently used options. --n <number>:: ---max-count=<number>:: +To control which revisions to show, gitk supports most options +applicable to the 'git rev-list' command. It also supports a few +options applicable to the 'git diff-*' commands to control how the +changes each commit introduces are shown. Finally, it supports some +gitk-specific options. + +gitk generally only understands options with arguments in the +'sticked' form (see linkgit:gitcli[7]) due to limitations in the +command line parser. + +rev-list options and arguments +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This manual page describes only the most frequently used options. See +linkgit:git-rev-list[1] for a complete list. + +--all:: + + Show all refs (branches, tags, etc.). - Limits the number of commits to show. +--branches[=<pattern>]:: +--tags[=<pattern>]:: +--remotes[=<pattern>]:: + + Pretend as if all the branches (tags, remote branches, resp.) + are listed on the command line as '<commit>'. If '<pattern>' + is given, limit refs to ones matching given shell glob. If + pattern lacks '?', '{asterisk}', or '[', '/{asterisk}' at the + end is implied. --since=<date>::
@@ -40,9 +57,9 @@ frequently used options. Show commits older than a specific date. ---all:: +--date-order:: - Show all branches. + Sort commits by date when possible. --merge::
@@ -51,19 +68,37 @@ frequently used options. that modify the conflicted files and do not exist on all the heads being merged. ---argscmd=<command>:: - Command to be run each time gitk has to determine the list of - <revs> to show. The command is expected to print on its standard - output a list of additional revs to be shown, one per line. - Use this instead of explicitly specifying <revs> if the set of - commits to show may vary between refreshes. +--left-right:: ---select-commit=<ref>:: + Mark which side of a symmetric diff a commit is reachable + from. Commits from the left side are prefixed with a `<` + symbol and those from the right with a `>` symbol. - Automatically select the specified commit after loading the graph. - Default behavior is equivalent to specifying '--select-commit=HEAD'. +--full-history:: + + When filtering history with '<path>...', does not prune some + history. (See "History simplification" in linkgit:git-log[1] + for a more detailed explanation.) + +--simplify-merges:: -<revs>:: + Additional option to '--full-history' to remove some needless + merges from the resulting history, as there are no selected + commits contributing to this merge. (See "History + simplification" in linkgit:git-log[1] for a more detailed + explanation.) + +--ancestry-path:: + + When given a range of commits to display + (e.g. 'commit1..commit2' or 'commit2 {caret}commit1'), only + display commits that exist directly on the ancestry chain + between the 'commit1' and 'commit2', i.e. commits that are + both descendants of 'commit1', and ancestors of 'commit2'. + (See "History simplification" in linkgit:git-log[1] for a more + detailed explanation.) + +<revision range>:: Limit the revisions to show. This can be either a single revision meaning show from the given revision and back, or it can be a range in
@@ -78,6 +113,23 @@ frequently used options. avoid ambiguity with respect to revision names use "--" to separate the paths from any preceding options. +gitk-specific options +~~~~~~~~~~~~~~~~~~~~~ + +--argscmd=<command>:: + + Command to be run each time gitk has to determine the revision + range to show. The command is expected to print on its + standard output a list of additional revisions to be shown, + one per line. Use this instead of explicitly specifying a + '<revision range>' if the set of commits to show may vary + between refreshes. + +--select-commit=<ref>:: + + Select the specified commit after loading the graph. + Default behavior is equivalent to specifying '--select-commit=HEAD'. + Examples -------- gitk v2.6.12.. include/scsi drivers/scsi::
@@ -101,6 +153,13 @@ Files Gitk creates the .gitk file in your $HOME directory to store preferences such as display options, font, and colors. +History +------- +Gitk was the first graphical repository browser. It's written in +tcl/tk and started off in a separate repository but was later merged +into the main Git repository. + + SEE ALSO -------- 'qgit(1)'::
Signature · 1 line
1.8.4.1.810.g312044e
PATCH v2 1/7 gitk: support -G option from the command line
From: Thomas Rast <[email protected]>
The -G option's usage is exactly analogous to that of -S, so supporting it is easy.
--- gitk-git/gitk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
gitk-git/gitk +1 -1
@@ -227,7 +227,7 @@ proc parseviewargs {n arglist} { "--until=*" - "--before=*" - "--max-age=*" - "--min-age=*" - "--author=*" - "--committer=*" - "--grep=*" - "-[iE]" - "--remove-empty" - "--first-parent" - "--cherry-pick" - - "-S*" - "--pickaxe-all" - "--pickaxe-regex" - + "-S*" - "-G*" - "--pickaxe-all" - "--pickaxe-regex" - "--simplify-by-decoration" { # These mean that we get a subset of the commits set filtered 1
Signature · 1 line
1.8.4.2.838.ga9a3e20
Re: [PATCH v2 1/7] gitk: support -G option from the command line
Quoted 8 lines · Thomas Rast
From: Thomas Rast <[email protected]>
The -G option's usage is exactly analogous to that of -S, so supporting it is easy.
Signed-off-by Thomas Rast <[email protected]>--- gitk-git/gitk | 2 +-
You CC'ed Paul, which is absolutely the right thing to do, but please make patches against his tree, which does not have gitk-git/ directory at the top-level.
I think the patch itself makes sense.
Thanks.
Quoted 15 lines
1 file changed, 1 insertion(+), 1 deletion(-)gitk-git/gitk +1
-1@@ -227,7 +227,7 @@ proc parseviewargs {n arglist} { "--until=*" - "--before=*" - "--max-age=*" - "--min-age=*" - "--author=*" - "--committer=*" - "--grep=*" - "-[iE]" - "--remove-empty" - "--first-parent" - "--cherry-pick" - - "-S*" - "--pickaxe-all" - "--pickaxe-regex" - + "-S*" - "-G*" - "--pickaxe-all" - "--pickaxe-regex" - "--simplify-by-decoration" { # These mean that we get a subset of the commits set filtered 1
Re: [PATCH v2 1/7] gitk: support -G option from the command line
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Thomas Rast
From: Thomas Rast <[email protected]>
The -G option's usage is exactly analogous to that of -S, so supporting it is easy.
Quoted 7 lines
Quoted 3 lines
Signed-off-by Thomas Rast <[email protected]>--- gitk-git/gitk | 2 +-You CC'ed Paul, which is absolutely the right thing to do, but please make patches against his tree, which does not have gitk-git/ directory at the top-level.
I figured this was easier on potential testers. As outlined in the cover letter, I'll do the splitting and rebasing later.
Signature · 2 lines
Thomas Rast [email protected]
Re: [PATCH v2 1/7] gitk: support -G option from the command line
Quoted 2 lines · Thomas Rast
I figured this was easier on potential testers. As outlined in the cover letter, I'll do the splitting and rebasing later.
Sorry, I should have read 0/7 before responding, but the message came out of order.
PATCH v2 2/7 gitk: refactor per-line part of getblobdiffline and its support
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
For later use with data sources other than a pipe, refactor the big worker part of getblobdiffline to a separate function parseblobdiffline. Also refactor its initialization and wrap-up to separate routines.
--- gitk-git/gitk | 408 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 213 insertions(+), 195 deletions(-)
gitk-git/gitk +213 -195
@@ -7710,15 +7710,25 @@ proc changeworddiff {name ix op} { reselectline } +proc initblobdiffvars {} { + global diffencoding targetline diffnparents + global diffinhdr currdiffsubmod diffseehere + set targetline {} + set diffnparents 0 + set diffinhdr 0 + set diffencoding [get_path_encoding {}] + set currdiffsubmod "" + set diffseehere -1 +} + proc getblobdiffs {ids} { global blobdifffd diffids env - global diffinhdr treediffs + global treediffs global diffcontext global ignorespace global worddiff global limitdiffs vfilelimit curview - global diffencoding targetline diffnparents - global git_version currdiffsubmod + global git_version set textconv {} if {[package vcompare $git_version "1.6.1"] >= 0} {
@@ -7742,13 +7752,9 @@ proc getblobdiffs {ids} { error_popup [mc "Error getting diffs: %s" $err] return } - set targetline {} - set diffnparents 0 - set diffinhdr 0 - set diffencoding [get_path_encoding {}] fconfigure $bdf -blocking 0 -encoding binary -eofchar {} set blobdifffd($ids) $bdf - set currdiffsubmod "" + initblobdiffvars filerun $bdf [list getblobdiffline $bdf $diffids] }
@@ -7814,13 +7820,17 @@ proc makediffhdr {fname ids} { set diffline 0 } +proc blobdiffmaybeseehere {ateof} { + global diffseehere + if {$diffseehere >= 0} { + mark_ctext_line [lindex [split $diffseehere .] 0] + } + maybe_scroll_ctext ateof +} + proc getblobdiffline {bdf ids} { - global diffids blobdifffd ctext curdiffstart - global diffnexthead diffnextnote difffilestart - global ctext_file_names ctext_file_lines - global diffinhdr treediffs mergemax diffnparents - global diffencoding jump_to_here targetline diffline currdiffsubmod - global worddiff + global diffids blobdifffd + global ctext set nr 0 $ctext conf -state normal
@@ -7829,212 +7839,220 @@ proc getblobdiffline {bdf ids} { catch {close $bdf} return 0 } - if {![string compare -length 5 "diff " $line]} { - if {![regexp {^diff (--cc|--git) } $line m type]} { - set line [encoding convertfrom $line] - $ctext insert end "$line\n" hunksep - continue + parseblobdiffline $ids $line + } + $ctext conf -state disabled + blobdiffmaybeseehere [eof $bdf] + if {[eof $bdf]} { + catch {close $bdf} + return 0 + } + return [expr {$nr >= 1000? 2: 1}] +} + +proc parseblobdiffline {ids line} { + global ctext curdiffstart + global diffnexthead diffnextnote difffilestart + global ctext_file_names ctext_file_lines + global diffinhdr treediffs mergemax diffnparents + global diffencoding jump_to_here targetline diffline currdiffsubmod + global worddiff diffseehere + + if {![string compare -length 5 "diff " $line]} { + if {![regexp {^diff (--cc|--git) } $line m type]} { + set line [encoding convertfrom $line] + $ctext insert end "$line\n" hunksep + continue + } + # start of a new file + set diffinhdr 1 + $ctext insert end "\n" + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names "" + lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + $ctext insert end "\n" filesep + + if {$type eq "--cc"} { + # start of a new file in a merge diff + set fname [string range $line 10 end] + if {[lsearch -exact $treediffs($ids) $fname] < 0} { + lappend treediffs($ids) $fname + add_flist [list $fname] } - # start of a new file - set diffinhdr 1 - $ctext insert end "\n" - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names "" - lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] - $ctext insert end "\n" filesep - - if {$type eq "--cc"} { - # start of a new file in a merge diff - set fname [string range $line 10 end] - if {[lsearch -exact $treediffs($ids) $fname] < 0} { - lappend treediffs($ids) $fname - add_flist [list $fname] - } + } else { + set line [string range $line 11 end] + # If the name hasn't changed the length will be odd, + # the middle char will be a space, and the two bits either + # side will be a/name and b/name, or "a/name" and "b/name". + # If the name has changed we'll get "rename from" and + # "rename to" or "copy from" and "copy to" lines following + # this, and we'll use them to get the filenames. + # This complexity is necessary because spaces in the + # filename(s) don't get escaped. + set l [string length $line] + set i [expr {$l / 2}] + if {!(($l & 1) && [string index $line $i] eq " " && + [string range $line 2 [expr {$i - 1}]] eq \ + [string range $line [expr {$i + 3}] end])} { + return + } + # unescape if quoted and chop off the a/ from the front + if {[string index $line 0] eq "\""} { + set fname [string range [lindex $line 0] 2 end] } else { - set line [string range $line 11 end] - # If the name hasn't changed the length will be odd, - # the middle char will be a space, and the two bits either - # side will be a/name and b/name, or "a/name" and "b/name". - # If the name has changed we'll get "rename from" and - # "rename to" or "copy from" and "copy to" lines following - # this, and we'll use them to get the filenames. - # This complexity is necessary because spaces in the - # filename(s) don't get escaped. - set l [string length $line] - set i [expr {$l / 2}] - if {!(($l & 1) && [string index $line $i] eq " " && - [string range $line 2 [expr {$i - 1}]] eq \ - [string range $line [expr {$i + 3}] end])} { - continue - } - # unescape if quoted and chop off the a/ from the front - if {[string index $line 0] eq "\""} { - set fname [string range [lindex $line 0] 2 end] - } else { - set fname [string range $line 2 [expr {$i - 1}]] - } + set fname [string range $line 2 [expr {$i - 1}]] } - makediffhdr $fname $ids + } + makediffhdr $fname $ids - } elseif {![string compare -length 16 "* Unmerged path " $line]} { - set fname [encoding convertfrom [string range $line 16 end]] - $ctext insert end "\n" - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names $fname - lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + } elseif {![string compare -length 16 "* Unmerged path " $line]} { + set fname [encoding convertfrom [string range $line 16 end]] + $ctext insert end "\n" + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names $fname + lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + $ctext insert end "$line\n" filesep + set i [lsearch -exact $treediffs($ids) $fname] + if {$i >= 0} { + setinlist difffilestart $i $curdiffstart + } + + } elseif {![string compare -length 2 "@@" $line]} { + regexp {^@@+} $line ats + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" hunksep + if {[regexp { \+(\d+),\d+ @@} $line m nl]} { + set diffline $nl + } + set diffnparents [expr {[string length $ats] - 1}] + set diffinhdr 0 + + } elseif {![string compare -length 10 "Submodule " $line]} { + # start of a new submodule + if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { + set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] + } else { + set fname [string range $line 10 [expr [string first "contains " $line] - 2]] + } + if {$currdiffsubmod != $fname} { + $ctext insert end "\n"; # Add newline after commit message + } + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names "" + if {$currdiffsubmod != $fname} { + lappend ctext_file_lines $fname + makediffhdr $fname $ids + set currdiffsubmod $fname + $ctext insert end "\n$line\n" filesep + } else { $ctext insert end "$line\n" filesep + } + } elseif {![string compare -length 3 " >" $line]} { + set $currdiffsubmod "" + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + } elseif {![string compare -length 3 " <" $line]} { + set $currdiffsubmod "" + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 + } elseif {$diffinhdr} { + if {![string compare -length 12 "rename from " $line]} { + set fname [string range $line [expr 6 + [string first " from " $line] ] end] + if {[string index $fname 0] eq "\""} { + set fname [lindex $fname 0] + } + set fname [encoding convertfrom $fname] set i [lsearch -exact $treediffs($ids) $fname] if {$i >= 0} { setinlist difffilestart $i $curdiffstart } - - } elseif {![string compare -length 2 "@@" $line]} { - regexp {^@@+} $line ats - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" hunksep - if {[regexp { \+(\d+),\d+ @@} $line m nl]} { - set diffline $nl + } elseif {![string compare -length 10 $line "rename to "] || + ![string compare -length 8 $line "copy to "]} { + set fname [string range $line [expr 4 + [string first " to " $line] ] end] + if {[string index $fname 0] eq "\""} { + set fname [lindex $fname 0] } - set diffnparents [expr {[string length $ats] - 1}] + makediffhdr $fname $ids + } elseif {[string compare -length 3 $line "---"] == 0} { + # do nothing + return + } elseif {[string compare -length 3 $line "+++"] == 0} { set diffinhdr 0 + return + } + $ctext insert end "$line\n" filesep - } elseif {![string compare -length 10 "Submodule " $line]} { - # start of a new submodule - if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { - set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] - } else { - set fname [string range $line 10 [expr [string first "contains " $line] - 2]] - } - if {$currdiffsubmod != $fname} { - $ctext insert end "\n"; # Add newline after commit message - } - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names "" - if {$currdiffsubmod != $fname} { - lappend ctext_file_lines $fname - makediffhdr $fname $ids - set currdiffsubmod $fname - $ctext insert end "\n$line\n" filesep - } else { - $ctext insert end "$line\n" filesep - } - } elseif {![string compare -length 3 " >" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 - } elseif {$diffinhdr} { - if {![string compare -length 12 "rename from " $line]} { - set fname [string range $line [expr 6 + [string first " from " $line] ] end] - if {[string index $fname 0] eq "\""} { - set fname [lindex $fname 0] - } - set fname [encoding convertfrom $fname] - set i [lsearch -exact $treediffs($ids) $fname] - if {$i >= 0} { - setinlist difffilestart $i $curdiffstart + } else { + set line [string map {\x1A ^Z} \ + [encoding convertfrom $diffencoding $line]] + # parse the prefix - one ' ', '-' or '+' for each parent + set prefix [string range $line 0 [expr {$diffnparents - 1}]] + set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] + set words_pre_markup "" + set words_post_markup "" + if {[string trim $prefix " -+"] eq {}} { + # prefix only has " ", "-" and "+" in it: normal diff line + set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } + if {$num >= 0} { + # removed line, first parent with line is $num + if {$num >= $mergemax} { + set num "max" } - } elseif {![string compare -length 10 $line "rename to "] || - ![string compare -length 8 $line "copy to "]} { - set fname [string range $line [expr 4 + [string first " to " $line] ] end] - if {[string index $fname 0] eq "\""} { - set fname [lindex $fname 0] + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "\[-$line-\]" $tag$num + } else { + $ctext insert end "$line" $tag$num } - makediffhdr $fname $ids - } elseif {[string compare -length 3 $line "---"] == 0} { - # do nothing - continue - } elseif {[string compare -length 3 $line "+++"] == 0} { - set diffinhdr 0 - continue - } - $ctext insert end "$line\n" filesep - - } else { - set line [string map {\x1A ^Z} \ - [encoding convertfrom $diffencoding $line]] - # parse the prefix - one ' ', '-' or '+' for each parent - set prefix [string range $line 0 [expr {$diffnparents - 1}]] - set tag [expr {$diffnparents > 1? "m": "d"}] - set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] - set words_pre_markup "" - set words_post_markup "" - if {[string trim $prefix " -+"] eq {}} { - # prefix only has " ", "-" and "+" in it: normal diff line - set num [string first "-" $prefix] - if {$dowords} { - set line [string range $line 1 end] + if {!$dowords} { + $ctext insert end "\n" $tag$num } - if {$num >= 0} { - # removed line, first parent with line is $num - if {$num >= $mergemax} { - set num "max" - } - if {$dowords && $worddiff eq [mc "Markup words"]} { - $ctext insert end "\[-$line-\]" $tag$num - } else { - $ctext insert end "$line" $tag$num - } - if {!$dowords} { - $ctext insert end "\n" $tag$num - } - } else { - set tags {} - if {[string first "+" $prefix] >= 0} { - # added line - lappend tags ${tag}result - if {$diffnparents > 1} { - set num [string first " " $prefix] - if {$num >= 0} { - if {$num >= $mergemax} { - set num "max" - } - lappend tags m$num + } else { + set tags {} + if {[string first "+" $prefix] >= 0} { + # added line + lappend tags ${tag}result + if {$diffnparents > 1} { + set num [string first " " $prefix] + if {$num >= 0} { + if {$num >= $mergemax} { + set num "max" } + lappend tags m$num } - set words_pre_markup "{+" - set words_post_markup "+}" } - if {$targetline ne {}} { - if {$diffline == $targetline} { - set seehere [$ctext index "end - 1 chars"] - set targetline {} - } else { - incr diffline - } - } - if {$dowords && $worddiff eq [mc "Markup words"]} { - $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + set words_pre_markup "{+" + set words_post_markup "+}" + } + if {$targetline ne {}} { + if {$diffline == $targetline} { + set diffseehere [$ctext index "end - 1 chars"] + set targetline {} } else { - $ctext insert end "$line" $tags - } - if {!$dowords} { - $ctext insert end "\n" $tags + incr diffline } } - } elseif {$dowords && $prefix eq "~"} { - $ctext insert end "\n" {} - } else { - # "\ No newline at end of file", - # or something else we don't recognize - $ctext insert end "$line\n" hunksep + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + } else { + $ctext insert end "$line" $tags + } + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} + } else { + # "\ No newline at end of file", + # or something else we don't recognize + $ctext insert end "$line\n" hunksep } } - if {[info exists seehere]} { - mark_ctext_line [lindex [split $seehere .] 0] - } - maybe_scroll_ctext [eof $bdf] - $ctext conf -state disabled - if {[eof $bdf]} { - catch {close $bdf} - return 0 - } - return [expr {$nr >= 1000? 2: 1}] } proc changediffdisp {} {
Signature · 1 line
1.8.4.2.838.ga9a3e20
PATCH v2 3/7 gitk: split out diff part in $commitinfo
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
So far we just parsed everything after the headers into the "comment" bit of $commitinfo, including notes and -- if you gave weird options -- the diff.
Split out the diff, if any, into a separate field. It's easy to recognize, since it always starts with /^diff/ and is preceded by an empty line.
We take care to snip away said empty line. The display code already properly spaces the end of the message from the first diff, and leaving another empty line at the end looks ugly.
--- gitk-git/gitk | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
gitk-git/gitk +10 -1
@@ -1704,8 +1704,17 @@ proc parsecommit {id contents listed} { set comment $newcomment } set hasnote [string first "\nNotes:\n" $contents] + set diff "" + # If there is diff output shown in the git-log stream, split it + # out. But get rid of the empty line that always precedes the + # diff. + set i [string first "\n\ndiff" $comment] + if {$i >= 0} { + set diff [string range $comment $i+1 end] + set comment [string range $comment 0 $i-1] + } set commitinfo($id) [list $headline $auname $audate \ - $comname $comdate $comment $hasnote] + $comname $comdate $comment $hasnote $diff] } proc getcommit {id} {
Signature · 1 line
1.8.4.2.838.ga9a3e20
PATCH v2 4/7 gitk: support showing the gathered inline diffs
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
The previous commit split the diffs into a separate field. Now we actually want to show them.
To that end we use the stored diff, and
- process it once to build a fake "tree diff", i.e., a list of all changed files;
- feed it through parseblobdiffline to actually format it into the $ctext field, like the existing diff machinery would.
--- gitk-git/gitk | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
gitk-git/gitk +38 -0
@@ -156,10 +156,12 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global vinlinediff global worddiff git_version set vdatemode($n) 0 set vmergeonly($n) 0 + set vinlinediff($n) 0 set glflags {} set diffargs {} set nextisval 0
@@ -7089,6 +7091,7 @@ proc selectline {l isnew {desired_loc {}}} { global cmitmode showneartags allcommits global targetrow targetid lastscrollrows global autoselect autosellen jump_to_here + global vinlinediff catch {unset pending_select} $canv delete hover
@@ -7230,6 +7233,8 @@ proc selectline {l isnew {desired_loc {}}} { init_flist [mc "Comments"] if {$cmitmode eq "tree"} { gettree $id + } elseif {$vinlinediff($curview) == 1} { + showinlinediff $id } elseif {[llength $olds] <= 1} { startdiff $id } else {
@@ -7566,6 +7571,39 @@ proc startdiff {ids} { } } +proc showinlinediff {ids} { + global commitinfo commitdata ctext + global treediffs + + set info $commitinfo($ids) + set diff [lindex $info 7] + set difflines [split $diff "\n"] + + initblobdiffvars + set treediff {} + + set inhdr 0 + foreach line $difflines { + if {![string compare -length 5 "diff " $line]} { + set inhdr 1 + } elseif {$inhdr && ![string compare -length 4 "+++ " $line]} { + # offset also accounts for the b/ prefix + lappend treediff [string range $line 6 end] + set inhdr 0 + } + } + + set treediffs($ids) $treediff + add_flist $treediff + + $ctext conf -state normal + foreach line $difflines { + parseblobdiffline $ids $line + } + maybe_scroll_ctext 1 + $ctext conf -state disabled +} + # If the filename (name) is under any of the passed filter paths # then return true to include the file in the listing. proc path_filter {filter name} {
Signature · 1 line
1.8.4.2.838.ga9a3e20
PATCH v2 5/7 gitk: recognize -L option
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
This gives line-log support to gitk, by exploiting the new support for processing and showing "inline" diffs straight from the git-log output.
Note that we 'set allknown 0', which is a bit counterintuitive since this is a "known" option. But that flag prevents gitk from thinking it can optimize the view by running rev-list to see the topology; in the -L case that doesn't work.
--- gitk-git/gitk | 8 ++++++++ 1 file changed, 8 insertions(+)
gitk-git/gitk +8 -0
@@ -235,6 +235,14 @@ proc parseviewargs {n arglist} { set filtered 1 lappend glflags $arg } + "-L*" { + # Line-log with 'sticked' argument (unsticked form is + # not supported) + set filtered 1 + set vinlinediff($n) 1 + set allknown 0 + lappend glflags $arg + } "-n" { # This appears to be the only one that has a value as a # separate word following it
Signature · 1 line
1.8.4.2.838.ga9a3e20
PATCH v2 6/7 Documentation: put blame/log -L in sticked form
In reply to Thomas RastThe next patch will document gitk -L, but gitk does not understand the separated form ('gitk -L :foo:bar' results in an error). Spell git-blame and git-log -L, which are supposed to be "the same" option, without the spaces to prevent confusion.
--- Documentation/blame-options.txt | 8 ++++---- Documentation/git-blame.txt | 8 ++++---- Documentation/git-log.txt | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-)
Documentation/blame-options.txt +4 -4
@@ -9,13 +9,13 @@ --show-stats:: Include additional statistics at the end of blame output. --L <start>,<end>:: --L :<regex>:: +-L<start>,<end>:: +-L:<regex>:: Annotate only the given line range. May be specified multiple times. Overlapping ranges are allowed. + -<start> and <end> are optional. ``-L <start>'' or ``-L <start>,'' spans from -<start> to end of file. ``-L ,<end>'' spans from start of file to <end>. +<start> and <end> are optional. ``-L<start>'' or ``-L<start>,'' spans from +<start> to end of file. ``-L,<end>'' spans from start of file to <end>. + include::line-range-format.txt[]
Documentation/git-blame.txt +4 -4
@@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] - [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] + [-L<range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>] [--] <file> DESCRIPTION
@@ -139,12 +139,12 @@ lines 40-60 for file `foo`, you can use the `-L` option like so (they mean the same thing -- both ask for 21 lines starting at line 40): - git blame -L 40,60 foo - git blame -L 40,+21 foo + git blame -L40,60 foo + git blame -L40,+21 foo Also you can use a regular expression to specify the line range: - git blame -L '/^sub hello {/,/^}$/' foo + git blame -L'/^sub hello {/,/^}$/' foo which limits the annotation to the body of the `hello` subroutine.
Documentation/git-log.txt +3 -3
@@ -62,8 +62,8 @@ produced by --stat etc. Note that only message is considered, if also a diff is shown its size is not included. --L <start>,<end>:<file>:: --L :<regex>:<file>:: +-L<start>,<end>:<file>:: +-L:<regex>:<file>:: Trace the evolution of the line range given by "<start>,<end>" (or the funcname regex <regex>) within the <file>. You may
@@ -152,7 +152,7 @@ EXAMPLES This makes sense only when following a strict policy of merging all topic branches when staying on a single integration branch. -`git log -L '/int main/',/^}/:main.c`:: +`git log -L'/int main/',/^}/:main.c`:: Shows how the function `main()` in the file 'main.c' evolved over time.
Signature · 1 line
1.8.4.2.838.ga9a3e20
Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
Quoted 4 lines · Thomas Rast
The next patch will document gitk -L, but gitk does not understand the separated form ('gitk -L :foo:bar' results in an error). Spell git-blame and git-log -L, which are supposed to be "the same" option, without the spaces to prevent confusion.
I agree that this patch may reduce confusion locally, but if we were to go in this direction, we should be consistent and enforce "stuck" form everywhere, not just the options you happened to have passed thru to gitk, but other options such as "-S <revs-file>", and also other commands that do not have anything to do with gitk (e.g. "git commit -C<commit>", not "git commit -C <commit>". Otherwise you will give a wrong impression to readers as if they have to remember which ones need to use the stuck form and which ones do not.
Quoted 82 lines
Signed-off-by Thomas Rast <[email protected]>--- Documentation/blame-options.txt | 8 ++++---- Documentation/git-blame.txt | 8 ++++---- Documentation/git-log.txt | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-)Documentation/blame-options.txt +4
-4@@ -9,13 +9,13 @@ --show-stats:: Include additional statistics at the end of blame output. --L <start>,<end>:: --L :<regex>:: +-L<start>,<end>:: +-L:<regex>:: Annotate only the given line range. May be specified multiple times. Overlapping ranges are allowed. + -<start> and <end> are optional. ``-L <start>'' or ``-L <start>,'' spans from -<start> to end of file. ``-L ,<end>'' spans from start of file to <end>. +<start> and <end> are optional. ``-L<start>'' or ``-L<start>,'' spans from +<start> to end of file. ``-L,<end>'' spans from start of file to <end>. + include::line-range-format.txt[]Documentation/git-blame.txt +4
-4@@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] - [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] + [-L<range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>] [--] <file> DESCRIPTION@@ -139,12 +139,12 @@ lines 40-60 for file `foo`, you can use the `-L` option like so (they mean the same thing -- both ask for 21 lines starting at line 40): - git blame -L 40,60 foo - git blame -L 40,+21 foo + git blame -L40,60 foo + git blame -L40,+21 foo Also you can use a regular expression to specify the line range: - git blame -L '/^sub hello {/,/^}$/' foo + git blame -L'/^sub hello {/,/^}$/' foo which limits the annotation to the body of the `hello` subroutine.Documentation/git-log.txt +3
-3@@ -62,8 +62,8 @@ produced by --stat etc. Note that only message is considered, if also a diff is shown its size is not included. --L <start>,<end>:<file>:: --L :<regex>:<file>:: +-L<start>,<end>:<file>:: +-L:<regex>:<file>:: Trace the evolution of the line range given by "<start>,<end>" (or the funcname regex <regex>) within the <file>. You may@@ -152,7 +152,7 @@ EXAMPLES This makes sense only when following a strict policy of merging all topic branches when staying on a single integration branch. -`git log -L '/int main/',/^}/:main.c`:: +`git log -L'/int main/',/^}/:main.c`:: Shows how the function `main()` in the file 'main.c' evolved over time.
Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
Quoted 7 lines · Junio C Hamano
Quoted 4 lines · Thomas Rast
The next patch will document gitk -L, but gitk does not understand the separated form ('gitk -L :foo:bar' results in an error). Spell git-blame and git-log -L, which are supposed to be "the same" option, without the spaces to prevent confusion.
Quoted 8 lines
I agree that this patch may reduce confusion locally, but if we were to go in this direction, we should be consistent and enforce "stuck" form everywhere, not just the options you happened to have passed thru to gitk, but other options such as "-S <revs-file>", and also other commands that do not have anything to do with gitk (e.g. "git commit -C<commit>", not "git commit -C <commit>". Otherwise you will give a wrong impression to readers as if they have to remember which ones need to use the stuck form and which ones do not.
Hmm. Do you want to go there?
(I can do it, but it'll obviously touch a lot of documentation.)
Signature · 2 lines
Thomas Rast [email protected]
Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
Quoted 18 lines · Thomas Rast
Quoted 7 lines · Junio C Hamano
Quoted 4 lines · Thomas Rast
The next patch will document gitk -L, but gitk does not understand the separated form ('gitk -L :foo:bar' results in an error). Spell git-blame and git-log -L, which are supposed to be "the same" option, without the spaces to prevent confusion.
Quoted 8 lines
I agree that this patch may reduce confusion locally, but if we were to go in this direction, we should be consistent and enforce "stuck" form everywhere, not just the options you happened to have passed thru to gitk, but other options such as "-S <revs-file>", and also other commands that do not have anything to do with gitk (e.g. "git commit -C<commit>", not "git commit -C <commit>". Otherwise you will give a wrong impression to readers as if they have to remember which ones need to use the stuck form and which ones do not.
Quoted 1 line
Hmm. Do you want to go there?
Absolutely not ;-)
But that unpleasant place would be the logical conclusion where this patch leads us to, I would have to say. I was hoping that there is an alternative solution to avoid that.
For example, gitk's parseviewargs is very well aware of the options it supports, and it goes through the argument list one by one, acting on what option it is looking at. Couldn't it be extended to handle options with stuck and unstuck form? After all, it has to know that "-L" and "-S" are supported options; it wouldn't be too much to ask for the parser to also know that "-L" eats the next token (i.e. pass the pair <"-L", next token> intact as two separate args to the underlying "log") while it can pass "-L?*" as is, no?
Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
Quoted 16 lines · Junio C Hamano
Quoted 7 lines · Thomas Rast
Quoted 3 lines · Junio C Hamano
I agree that this patch may reduce confusion locally, but if we were to go in this direction, we should be consistent and enforce "stuck" form everywhere,
Hmm. Do you want to go there?
Absolutely not ;-)
But that unpleasant place would be the logical conclusion where this patch leads us to, I would have to say. I was hoping that there is an alternative solution to avoid that.
Quoted 8 lines
For example, gitk's parseviewargs is very well aware of the options it supports, and it goes through the argument list one by one, acting on what option it is looking at. Couldn't it be extended to handle options with stuck and unstuck form? After all, it has to know that "-L" and "-S" are supported options; it wouldn't be too much to ask for the parser to also know that "-L" eats the next token (i.e. pass the pair <"-L", next token> intact as two separate args to the underlying "log") while it can pass "-L?*" as is, no?
It's not quite that easy because gitk does two-stage processing, and the big switch you are discussing here is only the second one. The first one is git-rev-parse, and while it happens to know about '-n 1', it does not recognize any other unstuck option arguments. (I haven't stared too long, but I think git-rev-parse is important to distinguish revisions from paths.)
I actually burned some train time today looking into this, and the situation is much worse than I thought. There is absolutely no consistency in any dimension:
a) many commands use parse_options internally, where mandatory args can be stuck or unstuck, but optional args must be stuck
a1) git branch --{contains,merged,no-merged} take a mandatory arg,
except if they are last on the command line, in which case the option reverts to the default (HEAD). Effectively this means the argument is half-optional but the spelling seen in the wild is usually unstuck.
a2) git-rev-parse (at least) still handrolls its parsing, so no
--default=HEAD
a3) git-commit-tree does not understand any of its short options in
stuck form (!)
b) the perl scripts mostly seem to be using Getopt::Long which handles things similarly, though I can't quote chapter&verse
b1) just to prove a point: git-add--interactive. I'm sure there's a
user-facing exception somewhere too...
c) shell scripts mostly go through git-sh-setup, using parseopt internally
c1) git-filter-branch
d) gitk doesn't do *un*stuck as explained above
On top of that, documentation is a wild mash of styles, sometimes even in the same manpage. For example, git-describe(1) tells the poor user about --candidates=<n> and four paragraphs further down about --match <pattern>.
So my short-term plan just became: document instead of fix; clean up manpages towards the stuck form for long options; have gitk only parse -Lstuck.
Medium term we can move gitk to a different option parser, resolving at least that inconsistency.
Longer term we can see about moving some more of the remaining craziness towards parseopt, getting consistency for free.
Signature · 2 lines
Thomas Rast [email protected]
Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
Quoted 9 lines · Thomas Rast
So my short-term plan just became: document instead of fix; clean up manpages towards the stuck form for long options; have gitk only parse -Lstuck.
Medium term we can move gitk to a different option parser, resolving at least that inconsistency.
Longer term we can see about moving some more of the remaining craziness towards parseopt, getting consistency for free.
In that sensible context, the patch sounds in line with the short-term step. Thanks.
v3 cover Documentation: stuck arguments and gitk log -L
In reply to Thomas RastThis gathers three rather separate patches.
1/3 is an unrelated fix for a string comparison issue that I noticed while surveying the state of stickiness.
2/3 is the "document" plan hinted at here:
Quoted 9 lines
So my short-term plan just became: document instead of fix; clean up manpages towards the stuck form for long options; have gitk only parse -Lstuck.
Medium term we can move gitk to a different option parser, resolving at least that inconsistency.
Longer term we can see about moving some more of the remaining craziness towards parseopt, getting consistency for free.
3/3 is the change to gitk(1) to match the 'gitk -L' support that I'm sending out separately (rebased to the gitk repo) and that will appear here:
http://mid.gmane.org/[email protected]
Of course it should only be applied once the corresponding code change is in place.
Thomas Rast (3):
commit-tree: use prefixcmp instead of memcmp(..., N)
Documentation: convert to --option=arg form where possible Documentation/gitk: document -L option
Documentation/blame-options.txt | 4 ++-- Documentation/fetch-options.txt | 2 +- Documentation/git-branch.txt | 6 +++--- Documentation/git-checkout.txt | 2 +- Documentation/git-cherry-pick.txt | 2 +- Documentation/git-clone.txt | 18 +++++++++--------- Documentation/git-config.txt | 4 ++-- Documentation/git-credential-cache.txt | 4 ++-- Documentation/git-cvsserver.txt | 2 +- Documentation/git-describe.txt | 2 +- Documentation/git-fmt-merge-msg.txt | 4 ++-- Documentation/git-format-patch.txt | 4 ++-- Documentation/git-grep.txt | 10 +++++----- Documentation/git-notes.txt | 2 +- Documentation/git-p4.txt | 14 +++++++------- Documentation/git-prune.txt | 4 ++-- Documentation/git-rebase.txt | 4 ++-- Documentation/git-replace.txt | 2 +- Documentation/git-revert.txt | 6 +++--- Documentation/gitk.txt | 16 ++++++++++++++++ builtin/commit-tree.c | 2 +- git-cvsserver.perl | 2 +- 22 files changed, 66 insertions(+), 50 deletions(-)
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 1/3 commit-tree: use prefixcmp instead of memcmp(..., N)
Handrolling the prefix comparison is harder to read and overruns if the argument is an empty string. Use our prefixcmp() instead.
--- builtin/commit-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
builtin/commit-tree.c +1 -1
@@ -61,7 +61,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)continue ; } -if (!memcmp (arg,"-S" ,2 )) { +if (!prefixcmp (arg,"-S" )) { sign_commit = arg +2 ;continue ; }
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 2/3 Documentation: convert to --option=arg form where possible
In reply to Thomas RastWe generally encourage the stuck form (that is, -oarg or --option=arg), for example gitcli(7) says
when a command line option takes an argument, use the stuck form [...] An option that takes optional option-argument must be written in the stuck form.
However the manpages do not express the options in this form in many places. Switch the long option descriptions to use --option=arg where possible. The short options are a more difficult choice, so this patch leaves them unchanged. In particular it's not clear whether the readability tradeoff is worth it for mandatory arguments, where -o foo is equally valid and sets 'foo' off visually from the option.
I found the possible matches by running
git grep '^--[^,]* .*::'
The following are not fixed by this patch:
* git-filter-branch
* git-quiltimport
* git-rev-parse
All of them have an ad-hoc parser that does not understand stuck forms.
The following are fixes beyond the pure choice of style:
* git-grep actually uses parseopt to take an optional argument for --open-files-in-pager. Therefore the unstuck form was not valid to begin with.
* In git-revert.txt, added <...> to signal that <parent-number> is a placeholder (all other options in the file already use this style).
--- Documentation/blame-options.txt | 4 ++-- Documentation/fetch-options.txt | 2 +- Documentation/git-branch.txt | 6 +++--- Documentation/git-checkout.txt | 2 +- Documentation/git-cherry-pick.txt | 2 +- Documentation/git-clone.txt | 18 +++++++++--------- Documentation/git-config.txt | 4 ++-- Documentation/git-credential-cache.txt | 4 ++-- Documentation/git-cvsserver.txt | 2 +- Documentation/git-describe.txt | 2 +- Documentation/git-fmt-merge-msg.txt | 4 ++-- Documentation/git-format-patch.txt | 4 ++-- Documentation/git-grep.txt | 10 +++++----- Documentation/git-notes.txt | 2 +- Documentation/git-p4.txt | 14 +++++++------- Documentation/git-prune.txt | 4 ++-- Documentation/git-rebase.txt | 4 ++-- Documentation/git-replace.txt | 2 +- Documentation/git-revert.txt | 6 +++--- git-cvsserver.perl | 2 +- 20 files changed, 49 insertions(+), 49 deletions(-)
Documentation/blame-options.txt +2 -2
@@ -55,14 +55,14 @@ include::line-range-format.txt[] discussion about encoding in the linkgit:git-log[1] manual page. ---contents <file>:: +--contents=<file>:: When <rev> is not specified, the command annotates the changes starting backwards from the working tree copy. This flag makes the command pretend as if the working tree copy has the contents of the named file (specify `-` to make the command read from the standard input). ---date <format>:: +--date=<format>:: The value is one of the following alternatives: {relative,local,default,iso,rfc,short}. If --date is not provided, the value of the blame.date config variable is
Documentation/fetch-options.txt +1 -1
@@ -109,7 +109,7 @@ endif::git-pull[] implementing your own Porcelain you are not supposed to use it. ---upload-pack <upload-pack>:: +--upload-pack=<upload-pack>:: When given, and the repository to fetch from is handled by 'git fetch-pack', '--exec=<upload-pack>' is passed to the command to specify non-default path for the command
Documentation/git-branch.txt +3 -3
@@ -195,15 +195,15 @@ start-point is either a local or remote-tracking branch. Open an editor and edit the text to explain what the branch is for, to be used by various other commands (e.g. `request-pull`). ---contains [<commit>]:: +--contains[=<commit>]:: Only list branches which contain the specified commit (HEAD if not specified). Implies `--list`. ---merged [<commit>]:: +--merged[=<commit>]:: Only list branches whose tips are reachable from the specified commit (HEAD if not specified). Implies `--list`. ---no-merged [<commit>]:: +--no-merged[=<commit>]:: Only list branches whose tips are not reachable from the specified commit (HEAD if not specified). Implies `--list`.
Documentation/git-checkout.txt +1 -1
@@ -157,7 +157,7 @@ explicitly give a name with '-b' in such a case. <commit> is not a branch name. See the "DETACHED HEAD" section below for details. ---orphan <new_branch>:: +--orphan=<new_branch>:: Create a new 'orphan' branch, named <new_branch>, started from <start_point> and switch to it. The first commit made on this new branch will have no parents and it will be the root of a new
Documentation/git-cherry-pick.txt +1 -1
@@ -76,7 +76,7 @@ OPTIONS default is not to do `-x` so this option is a no-op. -m parent-number:: ---mainline parent-number:: +--mainline=<parent-number>:: Usually you cannot cherry-pick a merge because you do not know which side of the merge should be considered the mainline. This option specifies the parent number (starting from 1) of
Documentation/git-clone.txt +9 -9
@@ -11,9 +11,9 @@ SYNOPSIS [verse] 'git clone' [--template=<template_directory>] [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror] - [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>] - [--separate-git-dir <git dir>] - [--depth <depth>] [--[no-]single-branch] + [-o <name>] [-b <name>] [-u <upload-pack>] [--reference=<repository>] + [--separate-git-dir=<git dir>] + [--depth=<depth>] [--[no-]single-branch] [--recursive | --recurse-submodules] [--] <repository> [<directory>]
@@ -93,7 +93,7 @@ If you want to break the dependency of a repository cloned with `-s` on its source repository, you can simply run `git repack -a` to copy all objects from the source repository into a pack in the cloned repository. ---reference <repository>:: +--reference=<repository>:: If the reference repository is on the local machine, automatically setup `.git/objects/info/alternates` to obtain objects from the reference repository. Using
@@ -144,12 +144,12 @@ objects from the source repository into a pack in the cloned repository. that all these refs are overwritten by a `git remote update` in the target repository. ---origin <name>:: +--origin=<name>:: -o <name>:: Instead of using the remote name `origin` to keep track of the upstream repository, use `<name>`. ---branch <name>:: +--branch=<name>:: -b <name>:: Instead of pointing the newly created HEAD to the branch pointed to by the cloned repository's HEAD, point to `<name>` branch
@@ -158,7 +158,7 @@ objects from the source repository into a pack in the cloned repository. `--branch` can also take tags and detaches the HEAD at that commit in the resulting repository. ---upload-pack <upload-pack>:: +--upload-pack=<upload-pack>:: -u <upload-pack>:: When given, and the repository to clone from is accessed via ssh, this specifies a non-default path for the command
@@ -168,7 +168,7 @@ objects from the source repository into a pack in the cloned repository. Specify the directory from which templates will be used; (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].) ---config <key>=<value>:: +--config=<key>=<value>:: -c <key>=<value>:: Set a configuration variable in the newly-created repository; this takes effect immediately after the repository is
@@ -179,7 +179,7 @@ objects from the source repository into a pack in the cloned repository. the config file. This makes it safe, for example, to add additional fetch refspecs to the origin remote. ---depth <depth>:: +--depth=<depth>:: Create a 'shallow' clone with a history truncated to the specified number of revisions. A shallow repository has a number of limitations (you cannot clone or fetch from
Documentation/git-config.txt +2 -2
@@ -135,10 +135,10 @@ from all available files. See also <<FILES>>. -f config-file:: ---file config-file:: +--file=config-file:: Use the given config file instead of the one specified by GIT_CONFIG. ---blob blob:: +--blob=blob:: Similar to '--file' but use the given blob instead of a file. E.g. you can use 'master:.gitmodules' to read values from the file '.gitmodules' in the master branch. See "SPECIFYING REVISIONS"
Documentation/git-credential-cache.txt +2 -2
@@ -26,11 +26,11 @@ linkgit:gitcredentials[7] or `EXAMPLES` below. OPTIONS ------- ---timeout <seconds>:: +--timeout=<seconds>:: Number of seconds to cache credentials (default: 900). ---socket <path>:: +--socket=<path>:: Use `<path>` to contact a running cache daemon (or start a new cache daemon if one is not started). Defaults to
Documentation/git-cvsserver.txt +1 -1
@@ -31,7 +31,7 @@ All these options obviously only make sense if enforced by the server side. They have been implemented to resemble the linkgit:git-daemon[1] options as closely as possible. ---base-path <path>:: +--base-path=<path>:: Prepend 'path' to requested CVSROOT --strict-paths::
Documentation/git-describe.txt +1 -1
@@ -80,7 +80,7 @@ OPTIONS describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2 that points at object deadbee....). ---match <pattern>:: +--match=<pattern>:: Only consider tags matching the given `glob(7)` pattern, excluding the "refs/tags/" prefix. This can be used to avoid leaking private tags from the repository.
Documentation/git-fmt-merge-msg.txt +2 -2
@@ -40,12 +40,12 @@ OPTIONS removed in the future. -m <message>:: ---message <message>:: +--message=<message>:: Use <message> instead of the branch names for the first line of the log message. For use with `--log`. -F <file>:: ---file <file>:: +--file=<file>:: Take the list of merged objects from <file> instead of stdin.
Documentation/git-format-patch.txt +2 -2
@@ -80,7 +80,7 @@ include::diff-options.txt[] Prepare patches from the topmost <n> commits. -o <dir>:: ---output-directory <dir>:: +--output-directory=<dir>:: Use <dir> to store the resulting files, instead of the current working directory.
@@ -92,7 +92,7 @@ include::diff-options.txt[] --no-numbered:: Name output in '[PATCH]' format. ---start-number <n>:: +--start-number=<n>:: Start numbering the patches at <n> instead of 1. --numbered-files::
Documentation/git-grep.txt +5 -5
@@ -95,7 +95,7 @@ OPTIONS -I:: Don't match the pattern in binary files. ---max-depth <depth>:: +--max-depth=<depth>:: For each <pathspec> given on command line, descend at most <depth> levels of directories. A negative value means no limit. This option is ignored if <pathspec> contains active wildcards.
@@ -158,7 +158,7 @@ OPTIONS synonym for `--files-with-matches`. -O [<pager>]:: ---open-files-in-pager [<pager>]:: +--open-files-in-pager[=<pager>]:: Open the matching files in the pager (not the output of 'grep'). If the pager happens to be "less" or "vi", and the user specified only one pattern, the first file is positioned at
@@ -200,17 +200,17 @@ OPTIONS -<num>:: -C <num>:: ---context <num>:: +--context=<num>:: Show <num> leading and trailing lines, and place a line containing `--` between contiguous groups of matches. -A <num>:: ---after-context <num>:: +--after-context=<num>:: Show <num> trailing lines, and place a line containing `--` between contiguous groups of matches. -B <num>:: ---before-context <num>:: +--before-context=<num>:: Show <num> leading lines, and place a line containing `--` between contiguous groups of matches.
Documentation/git-notes.txt +1 -1
@@ -155,7 +155,7 @@ OPTIONS Like '-C', but with '-c' the editor is invoked, so that the user can further edit the note message. ---ref <ref>:: +--ref=<ref>:: Manipulate the notes tree in <ref>. This overrides 'GIT_NOTES_REF' and the "core.notesRef" configuration. The ref is taken to be in `refs/notes/` if it is not qualified.
Documentation/git-p4.txt +7 -7
@@ -165,7 +165,7 @@ General options ~~~~~~~~~~~~~~~ All commands except clone accept these options. ---git-dir <dir>:: +--git-dir=<dir>:: Set the 'GIT_DIR' environment variable. See linkgit:git[1]. --verbose, -v::
@@ -176,7 +176,7 @@ Sync options These options can be used in the initial 'clone' as well as in subsequent 'sync' operations. ---branch <ref>:: +--branch=<ref>:: Import changes into <ref> instead of refs/remotes/p4/master. If <ref> starts with refs/, it is used as is. Otherwise, if it does not start with p4/, that prefix is added.
@@ -199,7 +199,7 @@ Git repository: Use the branch detection algorithm to find new paths in p4. It is documented below in "BRANCH DETECTION". ---changesfile <file>:: +--changesfile=<file>:: Import exactly the p4 change numbers listed in 'file', one per line. Normally, 'git p4' inspects the current p4 repository state and detects the changes it should import.
@@ -223,7 +223,7 @@ Git repository: sync operations must specify '--import-local' as well so that they can find the p4 branches in refs/heads. ---max-changes <n>:: +--max-changes=<n>:: Limit the number of imported changes to 'n'. Useful to limit the amount of history when using the '@all' p4 revision specifier.
@@ -245,7 +245,7 @@ Clone options These options can be used in an initial 'clone', along with the 'sync' options described above. ---destination <directory>:: +--destination=<directory>:: Where to create the Git repository. If not provided, the last component in the p4 depot path is used to create a new directory.
@@ -260,7 +260,7 @@ Submit options ~~~~~~~~~~~~~~ These options can be used to modify 'git p4 submit' behavior. ---origin <commit>:: +--origin=<commit>:: Upstream location from which commits are identified to submit to p4. By default, this is the most recent p4 commit reachable from 'HEAD'.
@@ -297,7 +297,7 @@ These options can be used to modify 'git p4 submit' behavior. to bypass the prompt, causing conflicting commits to be automatically skipped, or to quit trying to apply commits, without prompting. ---branch <branch>:: +--branch=<branch>:: After submitting, sync this named branch instead of the default p4/master. See the "Sync options" section above for more information.
Documentation/git-prune.txt +2 -2
@@ -9,7 +9,7 @@ git-prune - Prune all unreachable objects from the object database SYNOPSIS -------- [verse] -'git prune' [-n] [-v] [--expire <expire>] [--] [<head>...] +'git prune' [-n] [-v] [--expire=<expire>] [--] [<head>...] DESCRIPTION -----------
@@ -43,7 +43,7 @@ OPTIONS \--:: Do not interpret any more arguments as options. ---expire <time>:: +--expire=<time>:: Only expire loose objects older than <time>. <head>...::
Documentation/git-rebase.txt +2 -2
@@ -213,7 +213,7 @@ rebase.autostash:: OPTIONS ------- ---onto <newbase>:: +--onto=<newbase>:: Starting point at which to create the new commits. If the --onto option is not specified, the starting point is <upstream>. May be any valid commit, and not just an
@@ -351,7 +351,7 @@ with the `--interactive` option explicitly is generally not a good idea unless you know what you are doing (see BUGS below). -x <cmd>:: ---exec <cmd>:: +--exec=<cmd>:: Append "exec <cmd>" after each line creating a commit in the final history. <cmd> will be interpreted as one or more shell commands.
Documentation/git-replace.txt +1 -1
@@ -64,7 +64,7 @@ OPTIONS Delete existing replace refs for the given objects. -l <pattern>:: ---list <pattern>:: +--list=<pattern>:: List replace refs for objects that match the given pattern (or all if no pattern is given). Typing "git replace" without arguments, also lists all replace
Documentation/git-revert.txt +3 -3
@@ -8,7 +8,7 @@ git-revert - Revert some existing commits SYNOPSIS -------- [verse] -'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] <commit>... +'git revert' [--[no-]edit] [-n] [-m <parent-number>] [-s] <commit>... 'git revert' --continue 'git revert' --quit 'git revert' --abort
@@ -46,8 +46,8 @@ OPTIONS message prior to committing the revert. This is the default if you run the command from a terminal. --m parent-number:: ---mainline parent-number:: +-m <parent-number>:: +--mainline=<parent-number>:: Usually you cannot revert a merge because you do not know which side of the merge should be considered the mainline. This option specifies the parent number (starting from 1) of
git-cvsserver.perl +1 -1
@@ -108,7 +108,7 @@ my $usage = "usage: git cvsserver [options] [pserver|server] [<directory> ...]\n". - " --base-path <path> : Prepend to requested CVSROOT\n". + " --base-path=<path> : Prepend to requested CVSROOT\n". " Can be read from GIT_CVSSERVER_BASE_PATH\n". " --strict-paths : Don't allow recursing into subdirectories\n". " --export-all : Don't check for gitcvs.enabled in config\n".
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 3/3 Documentation/gitk: document -L option
In reply to Thomas RastThe -L option is the same as for git-log, so the entire block is just copied from git-log.txt. However, until the parser is fixed we add a caveat that gitk only understands the stuck form.
--- Documentation/gitk.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Documentation/gitk.txt +16 -0
@@ -98,6 +98,22 @@ linkgit:git-rev-list[1] for a complete list. (See "History simplification" in linkgit:git-log[1] for a more detailed explanation.) +-L<start>,<end>:<file>:: +-L:<regex>:<file>:: + + Trace the evolution of the line range given by "<start>,<end>" + (or the funcname regex <regex>) within the <file>. You may + not give any pathspec limiters. This is currently limited to + a walk starting from a single revision, i.e., you may only + give zero or one positive revision arguments. + You can specify this option more than once. ++ +*Note:* gitk (unlike linkgit:git-log[1]) currently only understands +this option if you specify it "glued together" with its argument. Do +*not* put a space after `-L`. ++ +include::line-range-format.txt[] + <revision range>:: Limit the revisions to show. This can be either a single revision
Signature · 1 line
1.8.5.rc2.348.gb73b695
v3 cover gitk -L
In reply to Thomas RastThese patches implement 'gitk -L'. They are exactly the same as the gitk patches from v2 at
http://thread.gmane.org/gmane.comp.version-control.git/227151/focus=236903
except that they apply to the gitk-git tree at
git://ozlabs.org/~paulus/gitk
The documentation change is in the parallel series that will appear at
http://mid.gmane.org/[email protected]
Thomas Rast (5):
gitk: support -G option from the command line
gitk: refactor per-line part of getblobdiffline and its support
gitk: split out diff part in $commitinfo
gitk: support showing the gathered inline diffs
gitk: recognize -L option
gitk | 467 +++++++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 270 insertions(+), 197 deletions(-)
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 1/5 gitk: support -G option from the command line
From: Thomas Rast <[email protected]>
The -G option's usage is exactly analogous to that of -S, so supporting it is easy.
--- gitk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
gitk +1 -1
@@ -227,7 +227,7 @@ proc parseviewargs {n arglist} { "--until=*" - "--before=*" - "--max-age=*" - "--min-age=*" - "--author=*" - "--committer=*" - "--grep=*" - "-[iE]" - "--remove-empty" - "--first-parent" - "--cherry-pick" - - "-S*" - "--pickaxe-all" - "--pickaxe-regex" - + "-S*" - "-G*" - "--pickaxe-all" - "--pickaxe-regex" - "--simplify-by-decoration" { # These mean that we get a subset of the commits set filtered 1
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 2/5 gitk: refactor per-line part of getblobdiffline and its support
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
For later use with data sources other than a pipe, refactor the big worker part of getblobdiffline to a separate function parseblobdiffline. Also refactor its initialization and wrap-up to separate routines.
--- gitk | 408 +++++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 213 insertions(+), 195 deletions(-)
gitk +213 -195
@@ -7710,15 +7710,25 @@ proc changeworddiff {name ix op} { reselectline } +proc initblobdiffvars {} { + global diffencoding targetline diffnparents + global diffinhdr currdiffsubmod diffseehere + set targetline {} + set diffnparents 0 + set diffinhdr 0 + set diffencoding [get_path_encoding {}] + set currdiffsubmod "" + set diffseehere -1 +} + proc getblobdiffs {ids} { global blobdifffd diffids env - global diffinhdr treediffs + global treediffs global diffcontext global ignorespace global worddiff global limitdiffs vfilelimit curview - global diffencoding targetline diffnparents - global git_version currdiffsubmod + global git_version set textconv {} if {[package vcompare $git_version "1.6.1"] >= 0} {
@@ -7742,13 +7752,9 @@ proc getblobdiffs {ids} { error_popup [mc "Error getting diffs: %s" $err] return } - set targetline {} - set diffnparents 0 - set diffinhdr 0 - set diffencoding [get_path_encoding {}] fconfigure $bdf -blocking 0 -encoding binary -eofchar {} set blobdifffd($ids) $bdf - set currdiffsubmod "" + initblobdiffvars filerun $bdf [list getblobdiffline $bdf $diffids] }
@@ -7814,13 +7820,17 @@ proc makediffhdr {fname ids} { set diffline 0 } +proc blobdiffmaybeseehere {ateof} { + global diffseehere + if {$diffseehere >= 0} { + mark_ctext_line [lindex [split $diffseehere .] 0] + } + maybe_scroll_ctext ateof +} + proc getblobdiffline {bdf ids} { - global diffids blobdifffd ctext curdiffstart - global diffnexthead diffnextnote difffilestart - global ctext_file_names ctext_file_lines - global diffinhdr treediffs mergemax diffnparents - global diffencoding jump_to_here targetline diffline currdiffsubmod - global worddiff + global diffids blobdifffd + global ctext set nr 0 $ctext conf -state normal
@@ -7829,212 +7839,220 @@ proc getblobdiffline {bdf ids} { catch {close $bdf} return 0 } - if {![string compare -length 5 "diff " $line]} { - if {![regexp {^diff (--cc|--git) } $line m type]} { - set line [encoding convertfrom $line] - $ctext insert end "$line\n" hunksep - continue + parseblobdiffline $ids $line + } + $ctext conf -state disabled + blobdiffmaybeseehere [eof $bdf] + if {[eof $bdf]} { + catch {close $bdf} + return 0 + } + return [expr {$nr >= 1000? 2: 1}] +} + +proc parseblobdiffline {ids line} { + global ctext curdiffstart + global diffnexthead diffnextnote difffilestart + global ctext_file_names ctext_file_lines + global diffinhdr treediffs mergemax diffnparents + global diffencoding jump_to_here targetline diffline currdiffsubmod + global worddiff diffseehere + + if {![string compare -length 5 "diff " $line]} { + if {![regexp {^diff (--cc|--git) } $line m type]} { + set line [encoding convertfrom $line] + $ctext insert end "$line\n" hunksep + continue + } + # start of a new file + set diffinhdr 1 + $ctext insert end "\n" + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names "" + lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + $ctext insert end "\n" filesep + + if {$type eq "--cc"} { + # start of a new file in a merge diff + set fname [string range $line 10 end] + if {[lsearch -exact $treediffs($ids) $fname] < 0} { + lappend treediffs($ids) $fname + add_flist [list $fname] } - # start of a new file - set diffinhdr 1 - $ctext insert end "\n" - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names "" - lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] - $ctext insert end "\n" filesep - - if {$type eq "--cc"} { - # start of a new file in a merge diff - set fname [string range $line 10 end] - if {[lsearch -exact $treediffs($ids) $fname] < 0} { - lappend treediffs($ids) $fname - add_flist [list $fname] - } + } else { + set line [string range $line 11 end] + # If the name hasn't changed the length will be odd, + # the middle char will be a space, and the two bits either + # side will be a/name and b/name, or "a/name" and "b/name". + # If the name has changed we'll get "rename from" and + # "rename to" or "copy from" and "copy to" lines following + # this, and we'll use them to get the filenames. + # This complexity is necessary because spaces in the + # filename(s) don't get escaped. + set l [string length $line] + set i [expr {$l / 2}] + if {!(($l & 1) && [string index $line $i] eq " " && + [string range $line 2 [expr {$i - 1}]] eq \ + [string range $line [expr {$i + 3}] end])} { + return + } + # unescape if quoted and chop off the a/ from the front + if {[string index $line 0] eq "\""} { + set fname [string range [lindex $line 0] 2 end] } else { - set line [string range $line 11 end] - # If the name hasn't changed the length will be odd, - # the middle char will be a space, and the two bits either - # side will be a/name and b/name, or "a/name" and "b/name". - # If the name has changed we'll get "rename from" and - # "rename to" or "copy from" and "copy to" lines following - # this, and we'll use them to get the filenames. - # This complexity is necessary because spaces in the - # filename(s) don't get escaped. - set l [string length $line] - set i [expr {$l / 2}] - if {!(($l & 1) && [string index $line $i] eq " " && - [string range $line 2 [expr {$i - 1}]] eq \ - [string range $line [expr {$i + 3}] end])} { - continue - } - # unescape if quoted and chop off the a/ from the front - if {[string index $line 0] eq "\""} { - set fname [string range [lindex $line 0] 2 end] - } else { - set fname [string range $line 2 [expr {$i - 1}]] - } + set fname [string range $line 2 [expr {$i - 1}]] } - makediffhdr $fname $ids + } + makediffhdr $fname $ids - } elseif {![string compare -length 16 "* Unmerged path " $line]} { - set fname [encoding convertfrom [string range $line 16 end]] - $ctext insert end "\n" - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names $fname - lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + } elseif {![string compare -length 16 "* Unmerged path " $line]} { + set fname [encoding convertfrom [string range $line 16 end]] + $ctext insert end "\n" + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names $fname + lappend ctext_file_lines [lindex [split $curdiffstart "."] 0] + $ctext insert end "$line\n" filesep + set i [lsearch -exact $treediffs($ids) $fname] + if {$i >= 0} { + setinlist difffilestart $i $curdiffstart + } + + } elseif {![string compare -length 2 "@@" $line]} { + regexp {^@@+} $line ats + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" hunksep + if {[regexp { \+(\d+),\d+ @@} $line m nl]} { + set diffline $nl + } + set diffnparents [expr {[string length $ats] - 1}] + set diffinhdr 0 + + } elseif {![string compare -length 10 "Submodule " $line]} { + # start of a new submodule + if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { + set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] + } else { + set fname [string range $line 10 [expr [string first "contains " $line] - 2]] + } + if {$currdiffsubmod != $fname} { + $ctext insert end "\n"; # Add newline after commit message + } + set curdiffstart [$ctext index "end - 1c"] + lappend ctext_file_names "" + if {$currdiffsubmod != $fname} { + lappend ctext_file_lines $fname + makediffhdr $fname $ids + set currdiffsubmod $fname + $ctext insert end "\n$line\n" filesep + } else { $ctext insert end "$line\n" filesep + } + } elseif {![string compare -length 3 " >" $line]} { + set $currdiffsubmod "" + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" dresult + } elseif {![string compare -length 3 " <" $line]} { + set $currdiffsubmod "" + set line [encoding convertfrom $diffencoding $line] + $ctext insert end "$line\n" d0 + } elseif {$diffinhdr} { + if {![string compare -length 12 "rename from " $line]} { + set fname [string range $line [expr 6 + [string first " from " $line] ] end] + if {[string index $fname 0] eq "\""} { + set fname [lindex $fname 0] + } + set fname [encoding convertfrom $fname] set i [lsearch -exact $treediffs($ids) $fname] if {$i >= 0} { setinlist difffilestart $i $curdiffstart } - - } elseif {![string compare -length 2 "@@" $line]} { - regexp {^@@+} $line ats - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" hunksep - if {[regexp { \+(\d+),\d+ @@} $line m nl]} { - set diffline $nl + } elseif {![string compare -length 10 $line "rename to "] || + ![string compare -length 8 $line "copy to "]} { + set fname [string range $line [expr 4 + [string first " to " $line] ] end] + if {[string index $fname 0] eq "\""} { + set fname [lindex $fname 0] } - set diffnparents [expr {[string length $ats] - 1}] + makediffhdr $fname $ids + } elseif {[string compare -length 3 $line "---"] == 0} { + # do nothing + return + } elseif {[string compare -length 3 $line "+++"] == 0} { set diffinhdr 0 + return + } + $ctext insert end "$line\n" filesep - } elseif {![string compare -length 10 "Submodule " $line]} { - # start of a new submodule - if {[regexp -indices "\[0-9a-f\]+\\.\\." $line nameend]} { - set fname [string range $line 10 [expr [lindex $nameend 0] - 2]] - } else { - set fname [string range $line 10 [expr [string first "contains " $line] - 2]] - } - if {$currdiffsubmod != $fname} { - $ctext insert end "\n"; # Add newline after commit message - } - set curdiffstart [$ctext index "end - 1c"] - lappend ctext_file_names "" - if {$currdiffsubmod != $fname} { - lappend ctext_file_lines $fname - makediffhdr $fname $ids - set currdiffsubmod $fname - $ctext insert end "\n$line\n" filesep - } else { - $ctext insert end "$line\n" filesep - } - } elseif {![string compare -length 3 " >" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" dresult - } elseif {![string compare -length 3 " <" $line]} { - set $currdiffsubmod "" - set line [encoding convertfrom $diffencoding $line] - $ctext insert end "$line\n" d0 - } elseif {$diffinhdr} { - if {![string compare -length 12 "rename from " $line]} { - set fname [string range $line [expr 6 + [string first " from " $line] ] end] - if {[string index $fname 0] eq "\""} { - set fname [lindex $fname 0] - } - set fname [encoding convertfrom $fname] - set i [lsearch -exact $treediffs($ids) $fname] - if {$i >= 0} { - setinlist difffilestart $i $curdiffstart + } else { + set line [string map {\x1A ^Z} \ + [encoding convertfrom $diffencoding $line]] + # parse the prefix - one ' ', '-' or '+' for each parent + set prefix [string range $line 0 [expr {$diffnparents - 1}]] + set tag [expr {$diffnparents > 1? "m": "d"}] + set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] + set words_pre_markup "" + set words_post_markup "" + if {[string trim $prefix " -+"] eq {}} { + # prefix only has " ", "-" and "+" in it: normal diff line + set num [string first "-" $prefix] + if {$dowords} { + set line [string range $line 1 end] + } + if {$num >= 0} { + # removed line, first parent with line is $num + if {$num >= $mergemax} { + set num "max" } - } elseif {![string compare -length 10 $line "rename to "] || - ![string compare -length 8 $line "copy to "]} { - set fname [string range $line [expr 4 + [string first " to " $line] ] end] - if {[string index $fname 0] eq "\""} { - set fname [lindex $fname 0] + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "\[-$line-\]" $tag$num + } else { + $ctext insert end "$line" $tag$num } - makediffhdr $fname $ids - } elseif {[string compare -length 3 $line "---"] == 0} { - # do nothing - continue - } elseif {[string compare -length 3 $line "+++"] == 0} { - set diffinhdr 0 - continue - } - $ctext insert end "$line\n" filesep - - } else { - set line [string map {\x1A ^Z} \ - [encoding convertfrom $diffencoding $line]] - # parse the prefix - one ' ', '-' or '+' for each parent - set prefix [string range $line 0 [expr {$diffnparents - 1}]] - set tag [expr {$diffnparents > 1? "m": "d"}] - set dowords [expr {$worddiff ne [mc "Line diff"] && $diffnparents == 1}] - set words_pre_markup "" - set words_post_markup "" - if {[string trim $prefix " -+"] eq {}} { - # prefix only has " ", "-" and "+" in it: normal diff line - set num [string first "-" $prefix] - if {$dowords} { - set line [string range $line 1 end] + if {!$dowords} { + $ctext insert end "\n" $tag$num } - if {$num >= 0} { - # removed line, first parent with line is $num - if {$num >= $mergemax} { - set num "max" - } - if {$dowords && $worddiff eq [mc "Markup words"]} { - $ctext insert end "\[-$line-\]" $tag$num - } else { - $ctext insert end "$line" $tag$num - } - if {!$dowords} { - $ctext insert end "\n" $tag$num - } - } else { - set tags {} - if {[string first "+" $prefix] >= 0} { - # added line - lappend tags ${tag}result - if {$diffnparents > 1} { - set num [string first " " $prefix] - if {$num >= 0} { - if {$num >= $mergemax} { - set num "max" - } - lappend tags m$num + } else { + set tags {} + if {[string first "+" $prefix] >= 0} { + # added line + lappend tags ${tag}result + if {$diffnparents > 1} { + set num [string first " " $prefix] + if {$num >= 0} { + if {$num >= $mergemax} { + set num "max" } + lappend tags m$num } - set words_pre_markup "{+" - set words_post_markup "+}" } - if {$targetline ne {}} { - if {$diffline == $targetline} { - set seehere [$ctext index "end - 1 chars"] - set targetline {} - } else { - incr diffline - } - } - if {$dowords && $worddiff eq [mc "Markup words"]} { - $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + set words_pre_markup "{+" + set words_post_markup "+}" + } + if {$targetline ne {}} { + if {$diffline == $targetline} { + set diffseehere [$ctext index "end - 1 chars"] + set targetline {} } else { - $ctext insert end "$line" $tags - } - if {!$dowords} { - $ctext insert end "\n" $tags + incr diffline } } - } elseif {$dowords && $prefix eq "~"} { - $ctext insert end "\n" {} - } else { - # "\ No newline at end of file", - # or something else we don't recognize - $ctext insert end "$line\n" hunksep + if {$dowords && $worddiff eq [mc "Markup words"]} { + $ctext insert end "$words_pre_markup$line$words_post_markup" $tags + } else { + $ctext insert end "$line" $tags + } + if {!$dowords} { + $ctext insert end "\n" $tags + } } + } elseif {$dowords && $prefix eq "~"} { + $ctext insert end "\n" {} + } else { + # "\ No newline at end of file", + # or something else we don't recognize + $ctext insert end "$line\n" hunksep } } - if {[info exists seehere]} { - mark_ctext_line [lindex [split $seehere .] 0] - } - maybe_scroll_ctext [eof $bdf] - $ctext conf -state disabled - if {[eof $bdf]} { - catch {close $bdf} - return 0 - } - return [expr {$nr >= 1000? 2: 1}] } proc changediffdisp {} {
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 3/5 gitk: split out diff part in $commitinfo
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
So far we just parsed everything after the headers into the "comment" bit of $commitinfo, including notes and -- if you gave weird options -- the diff.
Split out the diff, if any, into a separate field. It's easy to recognize, since it always starts with /^diff/ and is preceded by an empty line.
We take care to snip away said empty line. The display code already properly spaces the end of the message from the first diff, and leaving another empty line at the end looks ugly.
--- gitk | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
gitk +10 -1
@@ -1704,8 +1704,17 @@ proc parsecommit {id contents listed} { set comment $newcomment } set hasnote [string first "\nNotes:\n" $contents] + set diff "" + # If there is diff output shown in the git-log stream, split it + # out. But get rid of the empty line that always precedes the + # diff. + set i [string first "\n\ndiff" $comment] + if {$i >= 0} { + set diff [string range $comment $i+1 end] + set comment [string range $comment 0 $i-1] + } set commitinfo($id) [list $headline $auname $audate \ - $comname $comdate $comment $hasnote] + $comname $comdate $comment $hasnote $diff] } proc getcommit {id} {
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 4/5 gitk: support showing the gathered inline diffs
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
The previous commit split the diffs into a separate field. Now we actually want to show them.
To that end we use the stored diff, and
- process it once to build a fake "tree diff", i.e., a list of all changed files;
- feed it through parseblobdiffline to actually format it into the $ctext field, like the existing diff machinery would.
--- gitk | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
gitk +38 -0
@@ -156,10 +156,12 @@ proc unmerged_files {files} { proc parseviewargs {n arglist} { global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env + global vinlinediff global worddiff git_version set vdatemode($n) 0 set vmergeonly($n) 0 + set vinlinediff($n) 0 set glflags {} set diffargs {} set nextisval 0
@@ -7089,6 +7091,7 @@ proc selectline {l isnew {desired_loc {}}} { global cmitmode showneartags allcommits global targetrow targetid lastscrollrows global autoselect autosellen jump_to_here + global vinlinediff catch {unset pending_select} $canv delete hover
@@ -7230,6 +7233,8 @@ proc selectline {l isnew {desired_loc {}}} { init_flist [mc "Comments"] if {$cmitmode eq "tree"} { gettree $id + } elseif {$vinlinediff($curview) == 1} { + showinlinediff $id } elseif {[llength $olds] <= 1} { startdiff $id } else {
@@ -7566,6 +7571,39 @@ proc startdiff {ids} { } } +proc showinlinediff {ids} { + global commitinfo commitdata ctext + global treediffs + + set info $commitinfo($ids) + set diff [lindex $info 7] + set difflines [split $diff "\n"] + + initblobdiffvars + set treediff {} + + set inhdr 0 + foreach line $difflines { + if {![string compare -length 5 "diff " $line]} { + set inhdr 1 + } elseif {$inhdr && ![string compare -length 4 "+++ " $line]} { + # offset also accounts for the b/ prefix + lappend treediff [string range $line 6 end] + set inhdr 0 + } + } + + set treediffs($ids) $treediff + add_flist $treediff + + $ctext conf -state normal + foreach line $difflines { + parseblobdiffline $ids $line + } + maybe_scroll_ctext 1 + $ctext conf -state disabled +} + # If the filename (name) is under any of the passed filter paths # then return true to include the file in the listing. proc path_filter {filter name} {
Signature · 1 line
1.8.5.rc2.348.gb73b695
PATCH v3 5/5 gitk: recognize -L option
In reply to Thomas RastFrom: Thomas Rast <[email protected]>
This gives line-log support to gitk, by exploiting the new support for processing and showing "inline" diffs straight from the git-log output.
Note that we 'set allknown 0', which is a bit counterintuitive since this is a "known" option. But that flag prevents gitk from thinking it can optimize the view by running rev-list to see the topology; in the -L case that doesn't work.
--- gitk | 8 ++++++++ 1 file changed, 8 insertions(+)
gitk +8 -0
@@ -235,6 +235,14 @@ proc parseviewargs {n arglist} { set filtered 1 lappend glflags $arg } + "-L*" { + # Line-log with 'sticked' argument (unsticked form is + # not supported) + set filtered 1 + set vinlinediff($n) 1 + set allknown 0 + lappend glflags $arg + } "-n" { # This appears to be the only one that has a value as a # separate word following it
Signature · 1 line
1.8.5.rc2.348.gb73b695
Re: [PATCH v3 gitk 0/5] gitk -L
In reply to Thomas RastQuoted 8 lines · Thomas Rast
These patches implement 'gitk -L'. They are exactly the same as the gitk patches from v2 at
http://thread.gmane.org/gmane.comp.version-control.git/227151/focus=236903
except that they apply to the gitk-git tree at
git://ozlabs.org/~paulus/gitk
Thanks; applied all 5 (with s/sticked/stuck/g in patch 5).
Paul.
PATCH v2 7/7 Documentation/gitk: document -L option
In reply to Thomas RastThe -L option is the same as for git-log, so the entire block is just copied from git-log.txt.
--- Documentation/gitk.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Documentation/gitk.txt +12 -0
@@ -98,6 +98,18 @@ linkgit:git-rev-list[1] for a complete list. (See "History simplification" in linkgit:git-log[1] for a more detailed explanation.) +-L<start>,<end>:<file>:: +-L:<regex>:<file>:: + + Trace the evolution of the line range given by "<start>,<end>" + (or the funcname regex <regex>) within the <file>. You may + not give any pathspec limiters. This is currently limited to + a walk starting from a single revision, i.e., you may only + give zero or one positive revision arguments. + You can specify this option more than once. ++ +include::line-range-format.txt[] + <revision range>:: Limit the revisions to show. This can be either a single revision
Signature · 1 line
1.8.4.2.838.ga9a3e20
v2 cover gitk -L
In reply to Thomas RastQuoted 11 lines · Thomas Rast
Quoted 4 lines · Jonathan Nieder
Support for just the sticked form is better than nothing, especially if the gitk(1) manpage gains a note about it. In the long run I guess the ideal would be to add a parse-options-like library to the tcl support.
Ok. I'm generally not happy with the state of that manpage, so I took the chance to improve it.
[...]
I'll hold the gitk patches until we get this one sorted out, but then just do the sticked form as before.
Ok, there weren't any comments and this patch is in 'next', so here's gitk -L built on top of it.
I'm sending this as a single topic, but once agreement is reached, it has to be applied in two parts: the gitk patches (1-5) need to be subtree-rebased to match gitk.git, and then merged separately, while the documentation patches (6&7) need to be applied directly. The latter depend on the patch I'm replying to.
Changes since v1:
* 1/7 is new; while investigating the sticked/unsticked issue, I noticed that -G is not even supported by gitk.
* 3/7 has improved snipping logic, to get rid of an extra empty line before the diff
* 5/7 has a slightly different comment pointing out that this is only the sticked form of -L
* 6&7 are new, to resolve the sticky issue
Thomas Rast (7):
gitk: support -G option from the command line
gitk: refactor per-line part of getblobdiffline and its support
gitk: split out diff part in $commitinfo
gitk: support showing the gathered inline diffs
gitk: recognize -L option
Documentation: put blame/log -L in sticked form Documentation/gitk: document -L option
Documentation/blame-options.txt | 8 +- Documentation/git-blame.txt | 8 +- Documentation/git-log.txt | 6 +- Documentation/gitk.txt | 12 ++ gitk-git/gitk | 467 +++++++++++++++++++++++----------------- 5 files changed, 293 insertions(+), 208 deletions(-)
Signature · 1 line
1.8.4.2.838.ga9a3e20