gitk: Fix some off-by-one errors in computing which line to blame

When walking back from the line where a right-click happened to the
previous hunk separator line to calculate the line number to work on,
we were counting every line including the one clicked on.  That isn't
right; if the user clicked on the line immediately after the hunk
separator then the correct line number would be the one from the
hunk separator.  Therefore this looks at the clicked-on line to work
out which parent to blame (or whether to blame the current commit),
and then looks only at the preceding lines to work out the offset from
the line number in the hunk separator.

This also fixes an off-by-one error when we are showing files rather
than diffs.  In this case diff_menu_filebase is the line number of
the banner showing the file name, so the first line of the file is
at line $diff_menu_filebase + 1.

This also simplifies the code in find_hunk_blamespec a bit and arranges
that we don't pop up the context menu if the user clicks on a file
separator line or a hunk separator line.

Signed-off-by: Paul Mackerras <paulus@samba.org>

diff --git a/gitk b/gitk
index 00037976970d57888e2db09eec77d836b2215bbb..477590eed6a460eeeac0ff13b8b41fa3faac6948 100755 (executable)
--- a/gitk
+++ b/gitk
@@ -3019,9 +3019,13 @@ proc pop_diff_menu {w X Y x y} {
     global diff_menu_txtpos diff_menu_line
     global diff_menu_filebase
 
     global diff_menu_txtpos diff_menu_line
     global diff_menu_filebase
 

-    stopfinding

     set diff_menu_txtpos [split [$w index "@$x,$y"] "."]
     set diff_menu_line [lindex $diff_menu_txtpos 0]
     set diff_menu_txtpos [split [$w index "@$x,$y"] "."]
     set diff_menu_line [lindex $diff_menu_txtpos 0]

+    # don't pop up the menu on hunk-separator or file-separator lines
+    if {[lsearch -glob [$ctext tag names $diff_menu_line.0] "*sep"] >= 0} {
+       return
+    }
+    stopfinding

     set f [find_ctext_fileinfo $diff_menu_line]
     if {$f eq {}} return
     set flist_menu_file [lindex $f 0]
     set f [find_ctext_fileinfo $diff_menu_line]
     if {$f eq {}} return
     set flist_menu_file [lindex $f 0]


@@ -3159,45 +3163,47 @@ proc find_hunk_blamespec {base line} {
     }
 
     # Now scan the lines to determine offset within the hunk
     }
 
     # Now scan the lines to determine offset within the hunk

-    set parent {}

     set max_parent [expr {[llength $base_lines]-2}]
     set dline 0
     set s_lno [lindex [split $s_lix "."] 0]
 
     set max_parent [expr {[llength $base_lines]-2}]
     set dline 0
     set s_lno [lindex [split $s_lix "."] 0]
 

-    for {set i $line} {$i > $s_lno} {incr i -1} {
-       set c_line [$ctext get $i.0 "$i.0 + 1 lines"]
-       # Determine if the line is removed
-       set chunk [string range $c_line 0 $max_parent]
+    # 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
        set removed_idx [string first "-" $chunk]
        # Choose a parent index

-       if {$parent eq {}} {
-           if {$removed_idx >= 0} {
-               set parent $removed_idx
+       if {$removed_idx >= 0} {
+           set parent $removed_idx
+       } else {
+           set unchanged_idx [string first " " $chunk]
+           if {$unchanged_idx >= 0} {
+               set parent $unchanged_idx

            } else {
            } else {

-               set unchanged_idx [string first " " $chunk]
-               if {$unchanged_idx >= 0} {
-                   set parent $unchanged_idx
-               } else {
-                   # blame the current commit
-                   set parent -1
-               }
+               # blame the current commit
+               set parent -1

            }
        }
        # then count other lines that belong to it
            }
        }
        # then count other lines that belong to it

-       if {$parent >= 0} {
-           set code [string index $c_line $parent]
-           if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} {
-               incr dline
-           }
-       } else {
-           if {$removed_idx < 0} {
-               incr dline
+       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
+               }
+           } else {
+               if {$removed_idx < 0} {
+                   incr dline
+               }

            }
        }
            }
        }

+       incr parent
+    } else {
+       set parent 0

     }
 
     }
 

-    if {$parent eq {}} { set parent -1 }
-    incr parent

     incr dline [lindex $base_lines $parent]
     return [list $parent $dline]
 }
     incr dline [lindex $base_lines $parent]
     return [list $parent $dline]
 }


@@ -3209,7 +3215,7 @@ proc external_blame_diff {} {
 
     if {$cmitmode eq "tree"} {
        set parent_idx 0
 
     if {$cmitmode eq "tree"} {
        set parent_idx 0

-       set line [expr {$diff_menu_line - $diff_menu_filebase - 1}]
+       set line [expr {$diff_menu_line - $diff_menu_filebase}]

     } else {
        set hinfo [find_hunk_blamespec $diff_menu_filebase $diff_menu_line]
        if {$hinfo ne {}} {
     } else {
        set hinfo [find_hunk_blamespec $diff_menu_filebase $diff_menu_line]
        if {$hinfo ne {}} {