From: Paul Mackerras <paulus@samba.org>
Date: Sat, 25 Oct 2008 11:57:23 GMT
Subject: Re: [PATCH 3/7] gitk: Allow starting gui blame for a specific line.
Message-ID: <18691.2467.601039.480382@cargo.ozlabs.ibm.com>
In-Reply-To: <200810241213.01337.angavrilov@gmail.com>

Alexander Gavrilov writes:

> Good point. How about this variant? I renamed the menu item, and
> changed the code to blame the current commit if:

Sounds good, just a couple of minor nits...

> +    # 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"]
> +	# Determine if the line is removed
> +	set chunk [string range $c_line 0 [llength $base_lines]-2]

You need an [expr]:

set chunk [string range $c_line 0 [expr {[llength $base_lines] - 2}]]

> +	set removed_idx [string first "-" $chunk]
> +	# Choose a parent index
> +	if {$parent eq {}} {
> +	    if {$removed_idx >= 0} {
> +		set parent $removed_idx
> +		incr parent
> +	    } else {
> +		set unchanged_idx [string first " " $chunk]
> +		if {$unchanged_idx >= 0} {
> +		    set parent $unchanged_idx
> +		    incr parent
> +		} else {
> +		    # blame the current commit
> +		    set parent 0
> +		}
> +	    }
> +	}

I like this better than the previous version, but it would turn out a
bit simpler if you use parent = -1 to indicate that we're blaming the
current commit, and then increment it right at the end.

> +	# then count other lines that belong to it
> +	if {$parent > 0} {
> +	    set code [string index $c_line $parent-1]

Once again you need an [expr].

Apart from those things, it looks good.  If you like I will fix those
things and commit it.

Paul.