[PATCH] gitk: use single blamestuff for all show_line_source{} calls

[PATCH] gitk: use single blamestuff for all show_line_source{} calls

[Max Kirillov]

There seems to be no point to search for several origins at once.
I doubt it is even fully working (because there is one blameinst),
but blamestuff for some reason is an array. Also, it is not cleaned
after blame is completed

Signed-off-by: Max Kirillov <max@max630.net>
---
 gitk | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gitk b/gitk
index 90764e8..dfac4fd 100755
--- a/gitk
+++ b/gitk
@@ -3815,17 +3815,18 @@ proc show_line_source {} {
     nowbusy blaming [mc "Searching"]
     fconfigure $f -blocking 0
     set i [reg_instance $f]
-    set blamestuff($i) {}
+    set blamestuff {}
     set blameinst $i
     filerun $f [list read_line_source $f $i]
 }
 
 proc stopblaming {} {
-    global blameinst
+    global blameinst blamestuff
 
     if {[info exists blameinst]} {
 	stop_instance $blameinst
 	unset blameinst
+	unset blamestuff
 	notbusy blaming
     }
 }
@@ -3834,7 +3835,7 @@ proc read_line_source {fd inst} {
     global blamestuff curview commfd blameinst nullid nullid2
 
     while {[gets $fd line] >= 0} {
-	lappend blamestuff($inst) $line
+	lappend blamestuff $line
     }
     if {![eof $fd]} {
 	return 1
@@ -3845,17 +3846,18 @@ proc read_line_source {fd inst} {
     fconfigure $fd -blocking 1
     if {[catch {close $fd} err]} {
 	error_popup [mc "Error running git blame: %s" $err]
+	unset blamestuff
 	return 0
     }
 
     set fname {}
-    set line [split [lindex $blamestuff($inst) 0] " "]
+    set line [split [lindex $blamestuff 0] " "]
     set id [lindex $line 0]
     set lnum [lindex $line 1]
     if {[string length $id] == 40 && [string is xdigit $id] &&
 	[string is digit -strict $lnum]} {
 	# look for "filename" line
-	foreach l $blamestuff($inst) {
+	foreach l $blamestuff {
 	    if {[string match "filename *" $l]} {
 		set fname [string range $l 9 end]
 		break
@@ -3878,6 +3880,7 @@ proc read_line_source {fd inst} {
     } else {
 	puts "oops couldn't parse git blame output"
     }
+    unset blamestuff
     return 0
 }
 
-- 
1.8.5.2.421.g4cdf8d0

[PATCH 0/3] gitk: show latest change to region

[Max Kirillov]

Hi!

I quite like the "Show origin of this line" feature of the
gitk. It is more convenient than blame, because it
directly answers the question which is usually addressed to
blame.

But, sometimes there is no "key" line which one could blame.
Instead there is a function, block, or some other region of
code. So I need a similar feature, but for several lines
rather than for a single one.

The series of patches implements the feature. It consists of
3 patches:

* gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{}
* gitk: refactor: separate io from logic in the searching origin of line
* gitk: pick selection for region blame

It also requires the "gitk: use single blamestuff for all
show_line_source{} calls" patch, which it replies to.

-- 
Max

[PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{}

[Max Kirillov]

For requesting a region blame, it is necessary to parse a hunk and
find the region in the parent file corresponding to the selected region.
There is already hunk parsin functionality in the find_hunk_blamespec{},
but returns only information for a single line.

The new function, resolve_hunk_lines{}, scans the hunk once and returns
for all hunk lines between $start_diffline and $end_diffline, in which parent
each of them exists and which is its number there.

Signed-off-by: Max Kirillov <max@max630.net>
---
 gitk | 93 ++++++++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 57 insertions(+), 36 deletions(-)

diff --git a/gitk b/gitk
index dfac4fd..7699a66 100755
--- a/gitk
+++ b/gitk
@@ -3590,11 +3590,11 @@ proc external_diff {} {
     }
 }
 
-proc find_hunk_blamespec {base line} {
+proc resolve_hunk_lines {base start_diffline end_diffline} {
     global ctext
 
     # Find and parse the hunk header
-    set s_lix [$ctext search -backwards -regexp ^@@ "$line.0 lineend" $base.0]
+    set s_lix [$ctext search -backwards -regexp ^@@ "$start_diffline.0 lineend" $base.0]
     if {$s_lix eq {}} return
 
     set s_line [$ctext get $s_lix "$s_lix + 1 lines"]
@@ -3614,49 +3614,70 @@ proc find_hunk_blamespec {base line} {
     }
 
     # Now scan the lines to determine offset within the hunk
-    set max_parent [expr {[llength $base_lines]-2}]
-    set dline 0
+    set max_parent [expr {[llength $base_lines]-1}]
     set s_lno [lindex [split $s_lix "."] 0]
 
-    # Determine if the line is removed
-    set chunk [$ctext get $line.0 "$line.1 + $max_parent chars"]
-    if {[string match {[-+ ]*} $chunk]} {
-	set removed_idx [string first "-" $chunk]
-	# Choose a parent index
-	if {$removed_idx >= 0} {
-	    set parent $removed_idx
+    set commitlines_by_diffline {}
+    array unset commit_lines
+    for {set p 0} {$p <= $max_parent} {incr p} {
+	set commit_lines($p) [expr [lindex $base_lines $p] - 1]
+    }
+    for {set diffline [expr $s_lno + 1]} {$diffline <= $end_diffline} {incr diffline} {
+	set chunk [$ctext get $diffline.0 "$diffline.0 + $max_parent chars"]
+	if {$chunk eq {} || [string match "\[\n@\]*" $chunk]} {
+	    # region is larger than hunk
+	    return {}
+	}
+	set is_removed [expr [string first "-" $chunk] >= 0]
+	if {!$is_removed} {
+	    incr commit_lines(0)
+	    set commitlines [list [list 0 $commit_lines(0)]]
 	} else {
-	    set unchanged_idx [string first " " $chunk]
-	    if {$unchanged_idx >= 0} {
-		set parent $unchanged_idx
-	    } else {
-		# blame the current commit
-		set parent -1
-	    }
-	}
-	# then count other lines that belong to it
-	for {set i $line} {[incr i -1] > $s_lno} {} {
-	    set chunk [$ctext get $i.0 "$i.1 + $max_parent chars"]
-	    # Determine if the line is removed
-	    set removed_idx [string first "-" $chunk]
-	    if {$parent >= 0} {
-		set code [string index $chunk $parent]
-		if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} {
-		    incr dline
+	    set commitlines {}
+	}
+	for {set p 1} {$p <= $max_parent} {incr p} {
+	    switch -- [string index $chunk "$p-1"] {
+		"+" {
 		}
-	    } else {
-		if {$removed_idx < 0} {
-		    incr dline
+		"-" {
+		    incr commit_lines($p)
+		    lappend commitlines [list $p $commit_lines($p)]
+		}
+		" " {
+		    if {!$is_removed} {
+			incr commit_lines($p)
+			lappend commitlines [list $p $commit_lines($p)]
+		    }
+		}
+		default {
+		    error_popup "resolve_hunk_lines: unexpected diff line($diffline): $chunk"
+		    break
 		}
 	    }
 	}
-	incr parent
-    } else {
-	set parent 0
+	if {$diffline >= $start_diffline} {
+	    lappend commitlines_by_diffline [list $diffline $commitlines]
+	}
     }
+    return $commitlines_by_diffline
+}
 
-    incr dline [lindex $base_lines $parent]
-    return [list $parent $dline]
+proc find_hunk_blamespec {base line} {
+    foreach cl_spec [resolve_hunk_lines $base $line $line] {
+	if {[lindex $cl_spec 0] == $line} {
+	    set commitlines [lindex $cl_spec 1]
+	    if {[llength $commitlines] > 0} {
+		if {[llength $commitlines] > 1 && [lindex $commitlines 0 0] eq 0} {
+		    return [lindex $commitlines 1]
+		} else {
+		    return [lindex $commitlines 0]
+		}
+	    } else {
+		error_popup "find_hunk_blamespec: invalid commitlines: $commitlines"
+	    }
+	}
+    }
+    return {}
 }
 
 proc external_blame_diff {} {
-- 
1.8.5.2.421.g4cdf8d0

[PATCH 2/3] gitk: refactor: separate io from logic in the searching origin of line

[Max Kirillov]

The pattern of maintaining blame command and collecting output
can be reused for searching of latest change to region.
It still can use the blame's global variables, because the two
search commands should not run concurrently as well as two instances
of blame.

Signed-off-by: Max Kirillov <max@max630.net>
---
 gitk | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/gitk b/gitk
index 7699a66..eef88a1 100755
--- a/gitk
+++ b/gitk
@@ -3771,7 +3771,7 @@ proc external_blame {parent_idx {line {}}} {
 }
 
 proc show_line_source {} {
-    global cmitmode currentid parents curview blamestuff blameinst
+    global cmitmode currentid parents curview
     global diff_menu_line diff_menu_filebase flist_menu_file
     global nullid nullid2 gitdir cdup
 
@@ -3827,6 +3827,12 @@ proc show_line_source {} {
 	lappend blameargs $id
     }
     lappend blameargs -- [file join $cdup $flist_menu_file]
+    startblaming $blameargs read_line_source
+}
+
+proc startblaming {blameargs blamecommand} {
+    global blamestuff blameinst
+
     if {[catch {
 	set f [open $blameargs r]
     } err]} {
@@ -3838,7 +3844,7 @@ proc show_line_source {} {
     set i [reg_instance $f]
     set blamestuff {}
     set blameinst $i
-    filerun $f [list read_line_source $f $i]
+    filerun $f [list blameiocallback $f $i $blamecommand]
 }
 
 proc stopblaming {} {
@@ -3852,8 +3858,8 @@ proc stopblaming {} {
     }
 }
 
-proc read_line_source {fd inst} {
-    global blamestuff curview commfd blameinst nullid nullid2
+proc blameiocallback {fd inst blamecommand} {
+    global blamestuff blameinst commfd
 
     while {[gets $fd line] >= 0} {
 	lappend blamestuff $line
@@ -3871,6 +3877,14 @@ proc read_line_source {fd inst} {
 	return 0
     }
 
+    $blamecommand
+    unset blamestuff
+    return 0
+}
+
+proc read_line_source {} {
+    global blamestuff curview nullid nullid2
+
     set fname {}
     set line [split [lindex $blamestuff 0] " "]
     set id [lindex $line 0]
@@ -3901,7 +3915,6 @@ proc read_line_source {fd inst} {
     } else {
 	puts "oops couldn't parse git blame output"
     }
-    unset blamestuff
     return 0
 }
 
-- 
1.8.5.2.421.g4cdf8d0

[PATCH 3/3] gitk: pick selection for region blame

[Max Kirillov]

Add the new command to the diffmenu, "Show the latest change of selected
region".  The menu command picks selection, and if it exists and covers
a single hunk, locates the latest change which has been made to the
selected lines in the file.

The menu command is disabled if the region blame is impossible, for
example if nothing is selected or the selection does not lie fully in a
single diff hunk.

The search is implemented by use of "git log -L..." command. Unlike git
blame, it can locate merges which brings together changes to the region
from different branches. This is what is desired, actually.

Unfortunally, using git log -L for finding a single line origin is
suboptimal, because (a) it does not support the "--contents" commandline
argument, or any other way to blame uncommitted changes, and (b) it is
noticeably slower. Hopely in some future git log -L will be mature
enough to be used for picking the single line origin, for now the best
option is to implement region logic separately, reusing their basic io.

For diffs, the first parent is always searched. This decision is quite
voluntary, just to avoid complications to UI.

Signed-off-by: Max Kirillov <max@max630.net>
---
 gitk | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

diff --git a/gitk b/gitk
index eef88a1..676c990 100755
--- a/gitk
+++ b/gitk
@@ -2650,6 +2650,7 @@ proc makewindow {} {
     makemenu $diff_menu {
 	{mc "Show origin of this line" command show_line_source}
 	{mc "Run git gui blame on this line" command {external_blame_diff}}
+	{mc "Show the latest change of selected region" command show_selection_source}
     }
     $diff_menu configure -tearoff 0
 }
@@ -3464,6 +3465,7 @@ proc pop_diff_menu {w X Y x y} {
     global ctext diff_menu flist_menu_file
     global diff_menu_txtpos diff_menu_line
     global diff_menu_filebase
+    global selection_source_data
 
     set diff_menu_txtpos [split [$w index "@$x,$y"] "."]
     set diff_menu_line [lindex $diff_menu_txtpos 0]
@@ -3476,6 +3478,12 @@ proc pop_diff_menu {w X Y x y} {
     if {$f eq {}} return
     set flist_menu_file [lindex $f 0]
     set diff_menu_filebase [lindex $f 1]
+    prepare_show_selection_source
+    if {$selection_source_data ne {}} {
+	$diff_menu entryconf 2 -state normal
+    } else {
+	$diff_menu entryconf 2 -state disabled
+    }
     tk_popup $diff_menu $X $Y
 }
 
@@ -3918,6 +3926,136 @@ proc read_line_source {} {
     return 0
 }
 
+proc show_selection_source {} {
+    global selection_source_data
+    global blameinst blamestuff
+
+    if {$selection_source_data eq {}} {
+	return
+    }
+
+    foreach var {id fname lnum lnumber} val $selection_source_data { set $var $val }
+    set args [list | git log --pretty=format:%H "-L$lnum,+$lnumber:$fname" -M -n1 $id]
+
+    startblaming $args selsource_read
+}
+
+proc selsource_read {} {
+    global blamestuff
+    global curview
+
+    set id {}
+    set lnum {}
+    set state start
+    foreach l $blamestuff {
+	switch -- $state {
+	    start { if {[regexp {^([0-9a-f]{40})$} $l _ id_match]} {
+			set id $id_match
+			set state diff_header_diff
+		    } else {
+			break
+		    }
+	    }
+	    diff_header_diff { if {[regexp {^diff --git} $l]} { set state diff_header_oldfile } else { break } }
+	    diff_header_oldfile { if {[regexp {^---} $l]} {set state diff_header_newfile} else { break } }
+	    diff_header_newfile {
+		if {[regexp {^\+\+\+ b/(.*)$} $l _ fname_match]} {
+		    set fname $fname_match
+		    set state diff_hunk_header
+		} else {
+		    break
+		}
+	    }
+	    diff_hunk_header {
+		if {[regexp {^@@@*.* \+([0-9]+),[0-9]+ @@@*$} $l _ lnum_matched]} {
+		    set lnum $lnum_matched
+		    break
+		}
+	    }
+	}
+    }
+
+    if {$id eq {}} {
+	error_popup [mc "Parsing output of git log failed"]
+	return 0
+    }
+
+    if {![commitinview $id $curview]} {
+	error_popup [mc "The latest change is in commit %s, \
+			 which is not in this view" [shortids $id]]
+	return 0
+    }
+
+    selectline [rowofcommit $id] 1 [list $fname $lnum]
+}
+
+proc prepare_show_selection_source {} {
+    global ctext
+    global selection_source_data
+
+    set selection [$ctext tag ranges sel]
+    if {[llength $selection] != 2} {
+	set selection_source_data {}
+	return
+    }
+    set start_line [lindex [split [lindex $selection 0] "."] 0]
+    set end_index [split [lindex $selection 1] "."]
+    if {[lindex $end_index 1] eq 0} {
+	set end_line [expr [lindex $end_index 0] - 1]
+    } else {
+	set end_line [lindex $end_index 0]
+    }
+    set selection_source_data [prepare_show_region_source $start_line $end_line]
+}
+
+proc prepare_show_region_source {start_line end_line} {
+    global cmitmode
+    global currentid parents curview
+    global nullid
+    if {$start_line > $end_line} {
+	error_popup "Cannot blame region: $start_line > $end_line"
+	return {}
+    }
+    set f [find_ctext_fileinfo $start_line]
+    if {$f eq {}} {
+	return {}
+    }
+    if {$currentid eq $nullid} {
+	return {}
+    }
+    set fname [lindex $f 0]
+    set filebase [lindex $f 1]
+    if {$cmitmode eq "tree"} {
+	set id $currentid
+	set start_lnum [expr $start_line - $filebase]
+	set end_lnum [expr $end_line - $filebase]
+    } else {
+	set id [lindex $parents($curview,$currentid) 0]
+	set start_lnum {}
+	set end_lnum {}
+	foreach cl_spec [resolve_hunk_lines $filebase $start_line $end_line] {
+	    set diffline [lindex $cl_spec 0]
+	    if {$diffline > $end_line} {
+		break
+	    } elseif {$diffline >= $start_line} {
+		foreach branch [lindex $cl_spec 1] {
+		    if {[lindex $branch 0] == 1} {
+			if {$start_lnum eq {}} {
+			    set start_lnum [lindex $branch 1]
+			}
+			set end_lnum [lindex $branch 1]
+		    }
+		}
+	    }
+	}
+    }
+    if {$start_lnum ne {} && $end_lnum ne {}} {
+	return [list $id $fname $start_lnum [expr $end_lnum - $start_lnum + 1]]
+    } else {
+	return {}
+    }
+}
+
 # delete $dir when we see eof on $f (presumably because the child has exited)
 proc delete_at_eof {f dir} {
     while {[gets $f line] >= 0} {}
-- 
1.8.5.2.421.g4cdf8d0

[PATCH 3/3 v2] gitk: show latest change to region

[Max Kirillov]

Add a new command to the diffmenu, "Show the latest change of selected
region".  The menu command picks selection, and if it exists and covers
a single hunk, locates the latest change which has been made to the
selected lines in the file.

The menu command is disabled if the region blame is impossible, for
example if nothing is selected or the selection does not lie fully in a
single diff hunk.

The search is implemented by use of "git log -L..." command. Unlike git
blame, it can locate merges which brings together changes to the region
from different branches. This is what is desired, actually.

Unfortunally, using git log -L for finding a single line origin is
suboptimal, because (a) it does not support the "--contents" commandline
argument, or any other way to blame uncommitted changes, and (b) it is
noticeably slower. Hopely in some future git log -L will be mature
enough to be used for picking the single line origin, for now the best
option is to implement region logic separately, reusing the blame's basic io.

For diffs, the first parent is always searched. This decision is quite
voluntary, just to avoid complications to UI.

Signed-off-by: Max Kirillov <max@max630.net>
---
Fixed comment, same code
 gitk | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

diff --git a/gitk b/gitk
index eef88a1..676c990 100755
--- a/gitk
+++ b/gitk
@@ -2650,6 +2650,7 @@ proc makewindow {} {
     makemenu $diff_menu {
 	{mc "Show origin of this line" command show_line_source}
 	{mc "Run git gui blame on this line" command {external_blame_diff}}
+	{mc "Show the latest change of selected region" command show_selection_source}
     }
     $diff_menu configure -tearoff 0
 }
@@ -3464,6 +3465,7 @@ proc pop_diff_menu {w X Y x y} {
     global ctext diff_menu flist_menu_file
     global diff_menu_txtpos diff_menu_line
     global diff_menu_filebase
+    global selection_source_data
 
     set diff_menu_txtpos [split [$w index "@$x,$y"] "."]
     set diff_menu_line [lindex $diff_menu_txtpos 0]
@@ -3476,6 +3478,12 @@ proc pop_diff_menu {w X Y x y} {
     if {$f eq {}} return
     set flist_menu_file [lindex $f 0]
     set diff_menu_filebase [lindex $f 1]
+    prepare_show_selection_source
+    if {$selection_source_data ne {}} {
+	$diff_menu entryconf 2 -state normal
+    } else {
+	$diff_menu entryconf 2 -state disabled
+    }
     tk_popup $diff_menu $X $Y
 }
 
@@ -3918,6 +3926,136 @@ proc read_line_source {} {
     return 0
 }
 
+proc show_selection_source {} {
+    global selection_source_data
+    global blameinst blamestuff
+
+    if {$selection_source_data eq {}} {
+	return
+    }
+
+    foreach var {id fname lnum lnumber} val $selection_source_data { set $var $val }
+    set args [list | git log --pretty=format:%H "-L$lnum,+$lnumber:$fname" -M -n1 $id]
+
+    startblaming $args selsource_read
+}
+
+proc selsource_read {} {
+    global blamestuff
+    global curview
+
+    set id {}
+    set lnum {}
+    set state start
+    foreach l $blamestuff {
+	switch -- $state {
+	    start { if {[regexp {^([0-9a-f]{40})$} $l _ id_match]} {
+			set id $id_match
+			set state diff_header_diff
+		    } else {
+			break
+		    }
+	    }
+	    diff_header_diff { if {[regexp {^diff --git} $l]} { set state diff_header_oldfile } else { break } }
+	    diff_header_oldfile { if {[regexp {^---} $l]} {set state diff_header_newfile} else { break } }
+	    diff_header_newfile {
+		if {[regexp {^\+\+\+ b/(.*)$} $l _ fname_match]} {
+		    set fname $fname_match
+		    set state diff_hunk_header
+		} else {
+		    break
+		}
+	    }
+	    diff_hunk_header {
+		if {[regexp {^@@@*.* \+([0-9]+),[0-9]+ @@@*$} $l _ lnum_matched]} {
+		    set lnum $lnum_matched
+		    break
+		}
+	    }
+	}
+    }
+
+    if {$id eq {}} {
+	error_popup [mc "Parsing output of git log failed"]
+	return 0
+    }
+
+    if {![commitinview $id $curview]} {
+	error_popup [mc "The latest change is in commit %s, \
+			 which is not in this view" [shortids $id]]
+	return 0
+    }
+
+    selectline [rowofcommit $id] 1 [list $fname $lnum]
+}
+
+proc prepare_show_selection_source {} {
+    global ctext
+    global selection_source_data
+
+    set selection [$ctext tag ranges sel]
+    if {[llength $selection] != 2} {
+	set selection_source_data {}
+	return
+    }
+    set start_line [lindex [split [lindex $selection 0] "."] 0]
+    set end_index [split [lindex $selection 1] "."]
+    if {[lindex $end_index 1] eq 0} {
+	set end_line [expr [lindex $end_index 0] - 1]
+    } else {
+	set end_line [lindex $end_index 0]
+    }
+    set selection_source_data [prepare_show_region_source $start_line $end_line]
+}
+
+proc prepare_show_region_source {start_line end_line} {
+    global cmitmode
+    global currentid parents curview
+    global nullid
+    if {$start_line > $end_line} {
+	error_popup "Cannot blame region: $start_line > $end_line"
+	return {}
+    }
+    set f [find_ctext_fileinfo $start_line]
+    if {$f eq {}} {
+	return {}
+    }
+    if {$currentid eq $nullid} {
+	return {}
+    }
+    set fname [lindex $f 0]
+    set filebase [lindex $f 1]
+    if {$cmitmode eq "tree"} {
+	set id $currentid
+	set start_lnum [expr $start_line - $filebase]
+	set end_lnum [expr $end_line - $filebase]
+    } else {
+	set id [lindex $parents($curview,$currentid) 0]
+	set start_lnum {}
+	set end_lnum {}
+	foreach cl_spec [resolve_hunk_lines $filebase $start_line $end_line] {
+	    set diffline [lindex $cl_spec 0]
+	    if {$diffline > $end_line} {
+		break
+	    } elseif {$diffline >= $start_line} {
+		foreach branch [lindex $cl_spec 1] {
+		    if {[lindex $branch 0] == 1} {
+			if {$start_lnum eq {}} {
+			    set start_lnum [lindex $branch 1]
+			}
+			set end_lnum [lindex $branch 1]
+		    }
+		}
+	    }
+	}
+    }
+    if {$start_lnum ne {} && $end_lnum ne {}} {
+	return [list $id $fname $start_lnum [expr $end_lnum - $start_lnum + 1]]
+    } else {
+	return {}
+    }
+}
+
 # delete $dir when we see eof on $f (presumably because the child has exited)
 proc delete_at_eof {f dir} {
     while {[gets $f line] >= 0} {}
-- 
1.8.5.2.421.g4cdf8d0

Re: [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{}

[Eric Sunshine]

On Mon, Feb 3, 2014 at 5:41 PM, Max Kirillov <max@max630.net> wrote:
> For requesting a region blame, it is necessary to parse a hunk and
> find the region in the parent file corresponding to the selected region.
> There is already hunk parsin functionality in the find_hunk_blamespec{},

s/parsin/parsing/
s/in the/in/

> but returns only information for a single line.

s/but/but it/

> The new function, resolve_hunk_lines{}, scans the hunk once and returns
> for all hunk lines between $start_diffline and $end_diffline, in which parent
> each of them exists and which is its number there.
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
>  gitk | 93 ++++++++++++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 57 insertions(+), 36 deletions(-)
>
> diff --git a/gitk b/gitk
> index dfac4fd..7699a66 100755
> --- a/gitk
> +++ b/gitk
> @@ -3590,11 +3590,11 @@ proc external_diff {} {
>      }
>  }
>
> -proc find_hunk_blamespec {base line} {
> +proc resolve_hunk_lines {base start_diffline end_diffline} {
>      global ctext
>
>      # Find and parse the hunk header
> -    set s_lix [$ctext search -backwards -regexp ^@@ "$line.0 lineend" $base.0]
> +    set s_lix [$ctext search -backwards -regexp ^@@ "$start_diffline.0 lineend" $base.0]
>      if {$s_lix eq {}} return
>
>      set s_line [$ctext get $s_lix "$s_lix + 1 lines"]
> @@ -3614,49 +3614,70 @@ proc find_hunk_blamespec {base line} {
>      }
>
>      # Now scan the lines to determine offset within the hunk
> -    set max_parent [expr {[llength $base_lines]-2}]
> -    set dline 0
> +    set max_parent [expr {[llength $base_lines]-1}]
>      set s_lno [lindex [split $s_lix "."] 0]
>
> -    # Determine if the line is removed
> -    set chunk [$ctext get $line.0 "$line.1 + $max_parent chars"]
> -    if {[string match {[-+ ]*} $chunk]} {
> -       set removed_idx [string first "-" $chunk]
> -       # Choose a parent index
> -       if {$removed_idx >= 0} {
> -           set parent $removed_idx
> +    set commitlines_by_diffline {}
> +    array unset commit_lines
> +    for {set p 0} {$p <= $max_parent} {incr p} {
> +       set commit_lines($p) [expr [lindex $base_lines $p] - 1]
> +    }
> +    for {set diffline [expr $s_lno + 1]} {$diffline <= $end_diffline} {incr diffline} {
> +       set chunk [$ctext get $diffline.0 "$diffline.0 + $max_parent chars"]
> +       if {$chunk eq {} || [string match "\[\n@\]*" $chunk]} {
> +           # region is larger than hunk
> +           return {}
> +       }
> +       set is_removed [expr [string first "-" $chunk] >= 0]
> +       if {!$is_removed} {
> +           incr commit_lines(0)
> +           set commitlines [list [list 0 $commit_lines(0)]]
>         } else {
> -           set unchanged_idx [string first " " $chunk]
> -           if {$unchanged_idx >= 0} {
> -               set parent $unchanged_idx
> -           } else {
> -               # blame the current commit
> -               set parent -1
> -           }
> -       }
> -       # then count other lines that belong to it
> -       for {set i $line} {[incr i -1] > $s_lno} {} {
> -           set chunk [$ctext get $i.0 "$i.1 + $max_parent chars"]
> -           # Determine if the line is removed
> -           set removed_idx [string first "-" $chunk]
> -           if {$parent >= 0} {
> -               set code [string index $chunk $parent]
> -               if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} {
> -                   incr dline
> +           set commitlines {}
> +       }
> +       for {set p 1} {$p <= $max_parent} {incr p} {
> +           switch -- [string index $chunk "$p-1"] {
> +               "+" {
>                 }
> -           } else {
> -               if {$removed_idx < 0} {
> -                   incr dline
> +               "-" {
> +                   incr commit_lines($p)
> +                   lappend commitlines [list $p $commit_lines($p)]
> +               }
> +               " " {
> +                   if {!$is_removed} {
> +                       incr commit_lines($p)
> +                       lappend commitlines [list $p $commit_lines($p)]
> +                   }
> +               }
> +               default {
> +                   error_popup "resolve_hunk_lines: unexpected diff line($diffline): $chunk"
> +                   break
>                 }
>             }
>         }
> -       incr parent
> -    } else {
> -       set parent 0
> +       if {$diffline >= $start_diffline} {
> +           lappend commitlines_by_diffline [list $diffline $commitlines]
> +       }
>      }
> +    return $commitlines_by_diffline
> +}
>
> -    incr dline [lindex $base_lines $parent]
> -    return [list $parent $dline]
> +proc find_hunk_blamespec {base line} {
> +    foreach cl_spec [resolve_hunk_lines $base $line $line] {
> +       if {[lindex $cl_spec 0] == $line} {
> +           set commitlines [lindex $cl_spec 1]
> +           if {[llength $commitlines] > 0} {
> +               if {[llength $commitlines] > 1 && [lindex $commitlines 0 0] eq 0} {
> +                   return [lindex $commitlines 1]
> +               } else {
> +                   return [lindex $commitlines 0]
> +               }
> +           } else {
> +               error_popup "find_hunk_blamespec: invalid commitlines: $commitlines"
> +           }
> +       }
> +    }
> +    return {}
>  }
>
>  proc external_blame_diff {} {
> --
> 1.8.5.2.421.g4cdf8d0
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{}

[Max Kirillov]

On Mon, Feb 03, 2014 at 06:20:36PM -0500, Eric Sunshine wrote:
> On Mon, Feb 3, 2014 at 5:41 PM, Max Kirillov <max@max630.net> wrote:
>> For requesting a region blame, it is necessary to parse a hunk and
>> find the region in the parent file corresponding to the selected region.
>> There is already hunk parsin functionality in the find_hunk_blamespec{},
> 
> s/parsin/parsing/
> s/in the/in/
> 
>> but returns only information for a single line.
> 
> s/but/but it/
>

Thank you!

-- 
Max