All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH gitk 0/4] gitk support for git log -L
@ 2013-06-09 19:44 Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 1/4] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
                   ` (4 more replies)
  0 siblings, 5 replies; 42+ messages in thread
From: Thomas Rast @ 2013-06-09 19:44 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

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(-)

-- 
1.8.3.496.g0d0267b

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH gitk 1/4] gitk: refactor per-line part of getblobdiffline and its support
  2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
@ 2013-06-09 19:44 ` Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 2/4] gitk: split out diff part in $commitinfo Thomas Rast
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-06-09 19:44 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 408 +++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 213 insertions(+), 195 deletions(-)

diff --git a/gitk b/gitk
index 5cd00d8..81dce20 100755
--- a/gitk
+++ b/gitk
@@ -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 {} {
-- 
1.8.3.496.g0d0267b

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH gitk 2/4] gitk: split out diff part in $commitinfo
  2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 1/4] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
@ 2013-06-09 19:44 ` Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 3/4] gitk: support showing the gathered inline diffs Thomas Rast
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-06-09 19:44 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

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 the log message is indented but the /^diff / that
starts a diff is not.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 81dce20..261bda8 100755
--- a/gitk
+++ b/gitk
@@ -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} {
-- 
1.8.3.496.g0d0267b

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH gitk 3/4] gitk: support showing the gathered inline diffs
  2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 1/4] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 2/4] gitk: split out diff part in $commitinfo Thomas Rast
@ 2013-06-09 19:44 ` Thomas Rast
  2013-06-09 19:44 ` [PATCH gitk 4/4] gitk: recognize -L option Thomas Rast
  2013-07-23 15:19 ` [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
  4 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-06-09 19:44 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gitk b/gitk
index 261bda8..3715136 100755
--- a/gitk
+++ b/gitk
@@ -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} {
-- 
1.8.3.496.g0d0267b

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH gitk 4/4] gitk: recognize -L option
  2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
                   ` (2 preceding siblings ...)
  2013-06-09 19:44 ` [PATCH gitk 3/4] gitk: support showing the gathered inline diffs Thomas Rast
@ 2013-06-09 19:44 ` Thomas Rast
  2013-07-23 15:19 ` [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
  4 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-06-09 19:44 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gitk b/gitk
index 3715136..30474be 100755
--- a/gitk
+++ b/gitk
@@ -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
-- 
1.8.3.496.g0d0267b

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
                   ` (3 preceding siblings ...)
  2013-06-09 19:44 ` [PATCH gitk 4/4] gitk: recognize -L option Thomas Rast
@ 2013-07-23 15:19 ` Thomas Rast
  2013-07-29 19:37   ` Thomas Rast
  4 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-07-23 15:19 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

Thomas Rast <trast@inf.ethz.ch> writes:

> 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'?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-07-23 15:19 ` [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
@ 2013-07-29 19:37   ` Thomas Rast
  2013-07-29 20:07     ` Jens Lehmann
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-07-29 19:37 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Jens Lehmann

Thomas Rast <trast@inf.ethz.ch> writes:

> Thomas Rast <trast@inf.ethz.ch> writes:
>
>> 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.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-07-29 19:37   ` Thomas Rast
@ 2013-07-29 20:07     ` Jens Lehmann
  2013-07-31 13:17       ` Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Jens Lehmann @ 2013-07-29 20:07 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Paul Mackerras

Am 29.07.2013 21:37, schrieb Thomas Rast:
> Thomas Rast <trast@inf.ethz.ch> writes:
> 
>> Thomas Rast <trast@inf.ethz.ch> writes:
>>
>>> 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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-07-29 20:07     ` Jens Lehmann
@ 2013-07-31 13:17       ` Thomas Rast
  2013-08-18 11:54         ` Paul Mackerras
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-07-31 13:17 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: git, Paul Mackerras

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 29.07.2013 21:37, schrieb Thomas Rast:
>> Thomas Rast <trast@inf.ethz.ch> writes:
>> 
>>> Thomas Rast <trast@inf.ethz.ch> writes:
>>>
>>>> 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.

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.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-07-31 13:17       ` Thomas Rast
@ 2013-08-18 11:54         ` Paul Mackerras
  2013-08-19  8:21           ` Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Paul Mackerras @ 2013-08-18 11:54 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jens Lehmann, git

Hi Thomas,

On Wed, Jul 31, 2013 at 03:17:41PM +0200, Thomas Rast wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
> > Am 29.07.2013 21:37, schrieb Thomas Rast:
> >> Thomas Rast <trast@inf.ethz.ch> writes:
> >> 
> >>> Thomas Rast <trast@inf.ethz.ch> writes:
> >>>
> >>>> 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.

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.

> 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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-08-18 11:54         ` Paul Mackerras
@ 2013-08-19  8:21           ` Thomas Rast
  2013-08-19 17:30             ` Junio C Hamano
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-08-19  8:21 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Jens Lehmann, git

Paul Mackerras <paulus@samba.org> writes:

> Hi Thomas,
>
> On Wed, Jul 31, 2013 at 03:17:41PM +0200, Thomas Rast wrote:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> 
>> > Am 29.07.2013 21:37, schrieb Thomas Rast:
>> >> Thomas Rast <trast@inf.ethz.ch> writes:
>> >> 
>> >>> Thomas Rast <trast@inf.ethz.ch> writes:
>> >>>
>> >>>> Now that git log -L has hit master, I figure it's time to discuss the
>> >>>> corresponding change to gitk.
[...]
>
> 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.

>> 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'.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-08-19  8:21           ` Thomas Rast
@ 2013-08-19 17:30             ` Junio C Hamano
  2013-10-13  6:31               ` Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2013-08-19 17:30 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Paul Mackerras, Jens Lehmann, git

Thomas Rast <trast@inf.ethz.ch> writes:

> 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'.

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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-08-19 17:30             ` Junio C Hamano
@ 2013-10-13  6:31               ` Thomas Rast
  2013-10-14  5:25                 ` Jonathan Nieder
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-10-13  6:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul Mackerras, Jens Lehmann, git

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <trast@inf.ethz.ch> writes:
>
>> 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'.
>
> 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.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH gitk 0/4] gitk support for git log -L
  2013-10-13  6:31               ` Thomas Rast
@ 2013-10-14  5:25                 ` Jonathan Nieder
  2013-10-20 16:57                   ` [PATCH] Documentation: revamp gitk(1) Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Jonathan Nieder @ 2013-10-14  5:25 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, Paul Mackerras, Jens Lehmann, git

Thomas Rast wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>> Thomas Rast <trast@inf.ethz.ch> writes:

>>> 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.
>
> 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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH] Documentation: revamp gitk(1)
  2013-10-14  5:25                 ` Jonathan Nieder
@ 2013-10-20 16:57                   ` Thomas Rast
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-10-20 16:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---

Jonathan Nieder wrote:
> 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(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index c17e760..d44e14c 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -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)'::
-- 
1.8.4.1.810.g312044e

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 0/7] gitk -L
  2013-10-20 16:57                   ` [PATCH] Documentation: revamp gitk(1) Thomas Rast
@ 2013-10-29  7:20                     ` Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 1/7] gitk: support -G option from the command line Thomas Rast
                                         ` (6 more replies)
  0 siblings, 7 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann

Thomas Rast wrote:
> Jonathan Nieder wrote:
> > 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(-)

-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH v2 1/7] gitk: support -G option from the command line
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  2013-10-30  0:52                         ` Junio C Hamano
  2013-10-29  7:20                       ` [PATCH v2 2/7] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
                                         ` (5 subsequent siblings)
  6 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann,
	Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

The -G option's usage is exactly analogous to that of -S, so
supporting it is easy.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk-git/gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 5cd00d8..0e95814 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -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
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 2/7] gitk: refactor per-line part of getblobdiffline and its support
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 1/7] gitk: support -G option from the command line Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 3/7] gitk: split out diff part in $commitinfo Thomas Rast
                                         ` (4 subsequent siblings)
  6 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann,
	Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk-git/gitk | 408 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 213 insertions(+), 195 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 0e95814..11e988e 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -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 {} {
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 3/7] gitk: split out diff part in $commitinfo
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 1/7] gitk: support -G option from the command line Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 2/7] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 4/7] gitk: support showing the gathered inline diffs Thomas Rast
                                         ` (3 subsequent siblings)
  6 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann,
	Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk-git/gitk | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 11e988e..78b4354 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -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} {
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 4/7] gitk: support showing the gathered inline diffs
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
                                         ` (2 preceding siblings ...)
  2013-10-29  7:20                       ` [PATCH v2 3/7] gitk: split out diff part in $commitinfo Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 5/7] gitk: recognize -L option Thomas Rast
                                         ` (2 subsequent siblings)
  6 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann,
	Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk-git/gitk | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 78b4354..5ece2a1 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -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} {
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 5/7] gitk: recognize -L option
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
                                         ` (3 preceding siblings ...)
  2013-10-29  7:20                       ` [PATCH v2 4/7] gitk: support showing the gathered inline diffs Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 6/7] Documentation: put blame/log -L in sticked form Thomas Rast
  2013-10-29  7:20                       ` [PATCH v2 7/7] " Thomas Rast
  6 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann,
	Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk-git/gitk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 5ece2a1..3679467 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -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
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
                                         ` (4 preceding siblings ...)
  2013-10-29  7:20                       ` [PATCH v2 5/7] gitk: recognize -L option Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  2013-10-30  1:11                         ` Junio C Hamano
  2013-10-29  7:20                       ` [PATCH v2 7/7] " Thomas Rast
  6 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann

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.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 Documentation/blame-options.txt | 8 ++++----
 Documentation/git-blame.txt     | 8 ++++----
 Documentation/git-log.txt       | 6 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 0cebc4f..28ca95e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -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[]
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index f2c85cc..085dba1 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -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.
 
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 34097ef..87c10fa 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -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.
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v2 7/7] Documentation/gitk: document -L option
  2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
                                         ` (5 preceding siblings ...)
  2013-10-29  7:20                       ` [PATCH v2 6/7] Documentation: put blame/log -L in sticked form Thomas Rast
@ 2013-10-29  7:20                       ` Thomas Rast
  6 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-29  7:20 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jonathan Nieder, Paul Mackerras, Jens Lehmann

The -L option is the same as for git-log, so the entire block is just
copied from git-log.txt.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 Documentation/gitk.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index d44e14c..0f1c7f8 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -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
-- 
1.8.4.2.838.ga9a3e20

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 1/7] gitk: support -G option from the command line
  2013-10-29  7:20                       ` [PATCH v2 1/7] gitk: support -G option from the command line Thomas Rast
@ 2013-10-30  0:52                         ` Junio C Hamano
  2013-10-30  6:30                           ` Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2013-10-30  0:52 UTC (permalink / raw)
  To: Thomas Rast
  Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann, Thomas Rast

Thomas Rast <tr@thomasrast.ch> writes:

> From: Thomas Rast <trast@inf.ethz.ch>
>
> The -G option's usage is exactly analogous to that of -S, so
> supporting it is easy.
>
> Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
> ---
>  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.

>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 5cd00d8..0e95814 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
  2013-10-29  7:20                       ` [PATCH v2 6/7] Documentation: put blame/log -L in sticked form Thomas Rast
@ 2013-10-30  1:11                         ` Junio C Hamano
  2013-10-30  6:29                           ` Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2013-10-30  1:11 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann

Thomas Rast <tr@thomasrast.ch> writes:

> 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.

> Signed-off-by: Thomas Rast <tr@thomasrast.ch>
> ---
>  Documentation/blame-options.txt | 8 ++++----
>  Documentation/git-blame.txt     | 8 ++++----
>  Documentation/git-log.txt       | 6 +++---
>  3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
> index 0cebc4f..28ca95e 100644
> --- a/Documentation/blame-options.txt
> +++ b/Documentation/blame-options.txt
> @@ -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[]
>  
> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
> index f2c85cc..085dba1 100644
> --- a/Documentation/git-blame.txt
> +++ b/Documentation/git-blame.txt
> @@ -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.
>  
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 34097ef..87c10fa 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
  2013-10-30  1:11                         ` Junio C Hamano
@ 2013-10-30  6:29                           ` Thomas Rast
  2013-10-30 17:09                             ` Junio C Hamano
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-10-30  6:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <tr@thomasrast.ch> writes:
>
>> 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.

Hmm.  Do you want to go there?

(I can do it, but it'll obviously touch a lot of documentation.)

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 1/7] gitk: support -G option from the command line
  2013-10-30  0:52                         ` Junio C Hamano
@ 2013-10-30  6:30                           ` Thomas Rast
  2013-10-30 16:42                             ` Junio C Hamano
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Rast @ 2013-10-30  6:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann, Thomas Rast

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <tr@thomasrast.ch> writes:
>
>> From: Thomas Rast <trast@inf.ethz.ch>
>>
>> The -G option's usage is exactly analogous to that of -S, so
>> supporting it is easy.
>>
>> Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
>> ---
>>  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.

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 1/7] gitk: support -G option from the command line
  2013-10-30  6:30                           ` Thomas Rast
@ 2013-10-30 16:42                             ` Junio C Hamano
  0 siblings, 0 replies; 42+ messages in thread
From: Junio C Hamano @ 2013-10-30 16:42 UTC (permalink / raw)
  To: Thomas Rast
  Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann, Thomas Rast

Thomas Rast <tr@thomasrast.ch> writes:

> 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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
  2013-10-30  6:29                           ` Thomas Rast
@ 2013-10-30 17:09                             ` Junio C Hamano
  2013-10-30 18:59                               ` Thomas Rast
  0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2013-10-30 17:09 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann

Thomas Rast <tr@thomasrast.ch> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Thomas Rast <tr@thomasrast.ch> writes:
>>
>>> 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.
>
> 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?

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
  2013-10-30 17:09                             ` Junio C Hamano
@ 2013-10-30 18:59                               ` Thomas Rast
  2013-10-30 19:37                                 ` Junio C Hamano
                                                   ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Thomas Rast @ 2013-10-30 18:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <tr@thomasrast.ch> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> 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.
>
> 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.

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
  2013-10-30 18:59                               ` Thomas Rast
@ 2013-10-30 19:37                                 ` Junio C Hamano
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
  2013-11-16 17:37                                 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
  2 siblings, 0 replies; 42+ messages in thread
From: Junio C Hamano @ 2013-10-30 19:37 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Jonathan Nieder, Paul Mackerras, Jens Lehmann

Thomas Rast <tr@thomasrast.ch> writes:

> 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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH v3 gitk 0/5] gitk -L
  2013-10-30 18:59                               ` Thomas Rast
  2013-10-30 19:37                                 ` Junio C Hamano
@ 2013-11-16 17:37                                 ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 1/5] gitk: support -G option from the command line Thomas Rast
                                                     ` (5 more replies)
  2013-11-16 17:37                                 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
  2 siblings, 6 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann

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

The documentation change is in the parallel series that will appear at

  http://mid.gmane.org/cover.1384622379.git.tr@thomasrast.ch


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(-)

-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH v3 gitk 1/5] gitk: support -G option from the command line
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 2/5] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
                                                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann, Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

The -G option's usage is exactly analogous to that of -S, so
supporting it is easy.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 5cd00d8..0e95814 100755
--- a/gitk
+++ b/gitk
@@ -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
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 gitk 2/5] gitk: refactor per-line part of getblobdiffline and its support
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 1/5] gitk: support -G option from the command line Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 3/5] gitk: split out diff part in $commitinfo Thomas Rast
                                                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann, Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 408 +++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 213 insertions(+), 195 deletions(-)

diff --git a/gitk b/gitk
index 0e95814..11e988e 100755
--- a/gitk
+++ b/gitk
@@ -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 {} {
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 gitk 3/5] gitk: split out diff part in $commitinfo
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 1/5] gitk: support -G option from the command line Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 2/5] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 4/5] gitk: support showing the gathered inline diffs Thomas Rast
                                                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann, Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 11e988e..78b4354 100755
--- a/gitk
+++ b/gitk
@@ -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} {
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 gitk 4/5] gitk: support showing the gathered inline diffs
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
                                                     ` (2 preceding siblings ...)
  2013-11-16 17:37                                   ` [PATCH v3 gitk 3/5] gitk: split out diff part in $commitinfo Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 gitk 5/5] gitk: recognize -L option Thomas Rast
  2013-12-01 22:25                                   ` [PATCH v3 gitk 0/5] gitk -L Paul Mackerras
  5 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann, Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gitk b/gitk
index 78b4354..5ece2a1 100755
--- a/gitk
+++ b/gitk
@@ -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} {
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 gitk 5/5] gitk: recognize -L option
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
                                                     ` (3 preceding siblings ...)
  2013-11-16 17:37                                   ` [PATCH v3 gitk 4/5] gitk: support showing the gathered inline diffs Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-12-01 22:25                                   ` [PATCH v3 gitk 0/5] gitk -L Paul Mackerras
  5 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann, Thomas Rast

From: Thomas Rast <trast@inf.ethz.ch>

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.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 gitk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gitk b/gitk
index 5ece2a1..3679467 100755
--- a/gitk
+++ b/gitk
@@ -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
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L
  2013-10-30 18:59                               ` Thomas Rast
  2013-10-30 19:37                                 ` Junio C Hamano
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
@ 2013-11-16 17:37                                 ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 1/3] commit-tree: use prefixcmp instead of memcmp(..., N) Thomas Rast
                                                     ` (2 more replies)
  2 siblings, 3 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul Mackerras, Jonathan Nieder, Jens Lehmann, git

This 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:

> 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/cover.1384622392.git.tr@thomasrast.ch

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(-)

-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH v3 1/3] commit-tree: use prefixcmp instead of memcmp(..., N)
  2013-11-16 17:37                                 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 2/3] Documentation: convert to --option=arg form where possible Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 3/3] Documentation/gitk: document -L option Thomas Rast
  2 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul Mackerras, Jonathan Nieder, Jens Lehmann, git

Handrolling the prefix comparison is harder to read and overruns if
the argument is an empty string.  Use our prefixcmp() instead.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 builtin/commit-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index f641ff2..19d58f9 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -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;
 		}
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 2/3] Documentation: convert to --option=arg form where possible
  2013-11-16 17:37                                 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 1/3] commit-tree: use prefixcmp instead of memcmp(..., N) Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 3/3] Documentation/gitk: document -L option Thomas Rast
  2 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul Mackerras, Jonathan Nieder, Jens Lehmann, git

We 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).

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 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(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 0cebc4f..3d768f1 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -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
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index f0ef7d0..4a03e06 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -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
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 311b336..91fc23f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -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`.
 
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 91294f8..c95aed2 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -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
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index c205d23..7b54d88 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -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
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 450f158..a77f645 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -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
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e9917b8..6b21a3a 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -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"
diff --git a/Documentation/git-credential-cache.txt b/Documentation/git-credential-cache.txt
index 89b7306..5155dd3 100644
--- a/Documentation/git-credential-cache.txt
+++ b/Documentation/git-credential-cache.txt
@@ -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
diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 472f5cb..cd8fc0a 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -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::
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index d20ca40..4211c0e 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -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.
diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt
index bb1232a..415fda3 100644
--- a/Documentation/git-fmt-merge-msg.txt
+++ b/Documentation/git-fmt-merge-msg.txt
@@ -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.
 
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 5c0a4ab..9ada261 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -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::
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index f837334..ee474c1 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -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.
 
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 46ef046..0d23aaa 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -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.
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 8cba16d..04ea627 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -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.
diff --git a/Documentation/git-prune.txt b/Documentation/git-prune.txt
index bf82410..604c378 100644
--- a/Documentation/git-prune.txt
+++ b/Documentation/git-prune.txt
@@ -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>...::
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 94e07fd..35d121b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -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.
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index f373ab4..a82a36c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -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
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 2de67a5..3ddd242 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -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
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 95e69b1..cde9cc4 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -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".
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH v3 3/3] Documentation/gitk: document -L option
  2013-11-16 17:37                                 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 1/3] commit-tree: use prefixcmp instead of memcmp(..., N) Thomas Rast
  2013-11-16 17:37                                   ` [PATCH v3 2/3] Documentation: convert to --option=arg form where possible Thomas Rast
@ 2013-11-16 17:37                                   ` Thomas Rast
  2 siblings, 0 replies; 42+ messages in thread
From: Thomas Rast @ 2013-11-16 17:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Paul Mackerras, Jonathan Nieder, Jens Lehmann, git

The -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.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 Documentation/gitk.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index d44e14c..1e9e38a 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -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
-- 
1.8.5.rc2.348.gb73b695

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [PATCH v3 gitk 0/5] gitk -L
  2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
                                                     ` (4 preceding siblings ...)
  2013-11-16 17:37                                   ` [PATCH v3 gitk 5/5] gitk: recognize -L option Thomas Rast
@ 2013-12-01 22:25                                   ` Paul Mackerras
  5 siblings, 0 replies; 42+ messages in thread
From: Paul Mackerras @ 2013-12-01 22:25 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano, Jonathan Nieder, Jens Lehmann

On Sat, Nov 16, 2013 at 06:37:39PM +0100, Thomas Rast wrote:
> 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.

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2013-12-01 22:41 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 1/4] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 2/4] gitk: split out diff part in $commitinfo Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 3/4] gitk: support showing the gathered inline diffs Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 4/4] gitk: recognize -L option Thomas Rast
2013-07-23 15:19 ` [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
2013-07-29 19:37   ` Thomas Rast
2013-07-29 20:07     ` Jens Lehmann
2013-07-31 13:17       ` Thomas Rast
2013-08-18 11:54         ` Paul Mackerras
2013-08-19  8:21           ` Thomas Rast
2013-08-19 17:30             ` Junio C Hamano
2013-10-13  6:31               ` Thomas Rast
2013-10-14  5:25                 ` Jonathan Nieder
2013-10-20 16:57                   ` [PATCH] Documentation: revamp gitk(1) Thomas Rast
2013-10-29  7:20                     ` [PATCH v2 0/7] gitk -L Thomas Rast
2013-10-29  7:20                       ` [PATCH v2 1/7] gitk: support -G option from the command line Thomas Rast
2013-10-30  0:52                         ` Junio C Hamano
2013-10-30  6:30                           ` Thomas Rast
2013-10-30 16:42                             ` Junio C Hamano
2013-10-29  7:20                       ` [PATCH v2 2/7] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
2013-10-29  7:20                       ` [PATCH v2 3/7] gitk: split out diff part in $commitinfo Thomas Rast
2013-10-29  7:20                       ` [PATCH v2 4/7] gitk: support showing the gathered inline diffs Thomas Rast
2013-10-29  7:20                       ` [PATCH v2 5/7] gitk: recognize -L option Thomas Rast
2013-10-29  7:20                       ` [PATCH v2 6/7] Documentation: put blame/log -L in sticked form Thomas Rast
2013-10-30  1:11                         ` Junio C Hamano
2013-10-30  6:29                           ` Thomas Rast
2013-10-30 17:09                             ` Junio C Hamano
2013-10-30 18:59                               ` Thomas Rast
2013-10-30 19:37                                 ` Junio C Hamano
2013-11-16 17:37                                 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 gitk 1/5] gitk: support -G option from the command line Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 gitk 2/5] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 gitk 3/5] gitk: split out diff part in $commitinfo Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 gitk 4/5] gitk: support showing the gathered inline diffs Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 gitk 5/5] gitk: recognize -L option Thomas Rast
2013-12-01 22:25                                   ` [PATCH v3 gitk 0/5] gitk -L Paul Mackerras
2013-11-16 17:37                                 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 1/3] commit-tree: use prefixcmp instead of memcmp(..., N) Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 2/3] Documentation: convert to --option=arg form where possible Thomas Rast
2013-11-16 17:37                                   ` [PATCH v3 3/3] Documentation/gitk: document -L option Thomas Rast
2013-10-29  7:20                       ` [PATCH v2 7/7] " Thomas Rast

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.