Alexander Gavrilov writes:
> Adds a context menu item to the diff viewer pane that
> calls blame, focusing it on the clicked line. In case
> of combined diffs, it also automatically deduces which
> parent is to be blamed.
I have some comments on this one, but I haven't finished reviewing it
completely, so I may have more... :)
> +proc find_hunk_blamespec {base line} {
> + global ctext
> +
> + # Find and parse the hunk header
> + set s_lix [$ctext search -backwards -regexp ^@@ "$line.0 lineend" $base.0]
> + if {$s_lix eq {}} return
> +
> + set s_line [$ctext get $s_lix "$s_lix + 1 lines"]
> + if {![regexp {^@@@?(( -\d+(,\d+)?)+) \+(\d+)(,\d+)? @@} $s_line \
In fact there can be many @ characters at the beginning of the line;
the number of parents plus one, in fact. See commit
c465a76af658b443075d6efee1c3131257643020 in the kernel tree for an
example. I suggest starting with ^@@@* rather than ^@@@?.
> + # Now scan the lines to determine offset within the hunk
> + set parent {}
> + 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"]
> + if {$parent eq {}} {
> + # find first preceeding line that belongs to some parent
> + for {set j 0} {$j < [llength $old_lines]} {incr j} {
> + set code [string index $c_line $j]
> + if {$code ne {-} && $code ne { }} continue
> + if {$code eq { } && $parent ne {}} continue
> + set parent $j
> + if {$code eq {-}} break
> + }
> + }
This part worries me a bit. If the user clicks on a line where all
the $code values are "+" then I think we should blame the current
commit. Either that, or we disable the context menu item before
posting it if the user clicks on a line that starts with all "+"
characters (as many "+" as there are parents).
BTW, writing "-" rather than {-} in expressions is more idiomatic.
Paul.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
