Skip to content

Commit 190ec52

Browse files
committed
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 <[email protected]>
1 parent 7cdc355 commit 190ec52

File tree

1 file changed

+33
-27
lines changed

1 file changed

+33
-27
lines changed

gitk

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3019,9 +3019,13 @@ proc pop_diff_menu {w X Y x y} {
30193019
global diff_menu_txtpos diff_menu_line
30203020
global diff_menu_filebase
30213021

3022-
stopfinding
30233022
set diff_menu_txtpos [split [$w index "@$x,$y"] "."]
30243023
set diff_menu_line [lindex $diff_menu_txtpos 0]
3024+
# don't pop up the menu on hunk-separator or file-separator lines
3025+
if {[lsearch -glob [$ctext tag names $diff_menu_line.0] "*sep"] >= 0} {
3026+
return
3027+
}
3028+
stopfinding
30253029
set f [find_ctext_fileinfo $diff_menu_line]
30263030
if {$f eq {}} return
30273031
set flist_menu_file [lindex $f 0]
@@ -3159,45 +3163,47 @@ proc find_hunk_blamespec {base line} {
31593163
}
31603164

31613165
# Now scan the lines to determine offset within the hunk
3162-
set parent {}
31633166
set max_parent [expr {[llength $base_lines]-2}]
31643167
set dline 0
31653168
set s_lno [lindex [split $s_lix "."] 0]
31663169

3167-
for {set i $line} {$i > $s_lno} {incr i -1} {
3168-
set c_line [$ctext get $i.0 "$i.0 + 1 lines"]
3169-
# Determine if the line is removed
3170-
set chunk [string range $c_line 0 $max_parent]
3170+
# Determine if the line is removed
3171+
set chunk [$ctext get $line.0 "$line.1 + $max_parent chars"]
3172+
if {[string match {[-+ ]*} $chunk]} {
31713173
set removed_idx [string first "-" $chunk]
31723174
# Choose a parent index
3173-
if {$parent eq {}} {
3174-
if {$removed_idx >= 0} {
3175-
set parent $removed_idx
3175+
if {$removed_idx >= 0} {
3176+
set parent $removed_idx
3177+
} else {
3178+
set unchanged_idx [string first " " $chunk]
3179+
if {$unchanged_idx >= 0} {
3180+
set parent $unchanged_idx
31763181
} else {
3177-
set unchanged_idx [string first " " $chunk]
3178-
if {$unchanged_idx >= 0} {
3179-
set parent $unchanged_idx
3180-
} else {
3181-
# blame the current commit
3182-
set parent -1
3183-
}
3182+
# blame the current commit
3183+
set parent -1
31843184
}
31853185
}
31863186
# then count other lines that belong to it
3187-
if {$parent >= 0} {
3188-
set code [string index $c_line $parent]
3189-
if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} {
3190-
incr dline
3191-
}
3192-
} else {
3193-
if {$removed_idx < 0} {
3194-
incr dline
3187+
for {set i $line} {[incr i -1] > $s_lno} {} {
3188+
set chunk [$ctext get $i.0 "$i.1 + $max_parent chars"]
3189+
# Determine if the line is removed
3190+
set removed_idx [string first "-" $chunk]
3191+
if {$parent >= 0} {
3192+
set code [string index $chunk $parent]
3193+
if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} {
3194+
incr dline
3195+
}
3196+
} else {
3197+
if {$removed_idx < 0} {
3198+
incr dline
3199+
}
31953200
}
31963201
}
3202+
incr parent
3203+
} else {
3204+
set parent 0
31973205
}
31983206

3199-
if {$parent eq {}} { set parent -1 }
3200-
incr parent
32013207
incr dline [lindex $base_lines $parent]
32023208
return [list $parent $dline]
32033209
}
@@ -3209,7 +3215,7 @@ proc external_blame_diff {} {
32093215

32103216
if {$cmitmode eq "tree"} {
32113217
set parent_idx 0
3212-
set line [expr {$diff_menu_line - $diff_menu_filebase - 1}]
3218+
set line [expr {$diff_menu_line - $diff_menu_filebase}]
32133219
} else {
32143220
set hinfo [find_hunk_blamespec $diff_menu_filebase $diff_menu_line]
32153221
if {$hinfo ne {}} {

0 commit comments

Comments
 (0)