Re: [PATCH 1/2] gitk: Add a 'rename' option to the branch context menu

From: Paul Mackerras <paulus@ozlabs.org>
Date: Mar 19, 2016, 03:45 UTC
Message-ID: <20160319034555.GB27126@fergus.ozlabs.ibm.com>
In-Reply-To: <1859807.50qitjY8Ul@wiske>

On Fri, Jan 15, 2016 at 10:38:49PM +0100, Rogier Goossens wrote:

> 
> Signed-off-by: Rogier Goossens <goossens.rogier@gmail.com>

This is a nice idea; I just have some comments about the Tcl here:

Show 11 quoted lines

> @@ -9756,15 +9831,19 @@ proc headmenu {x y id head} {
>      stopfinding
>      set headmenuid $id
>      set headmenuhead $head
> -    set state normal
> +    array set state {0 normal 1 normal 2 normal 3 normal}
>      if {[string match "remotes/*" $head]} {
> -	set state disabled
> +	set state(0) disabled
> +	set state(1) disabled
> +	set state(2) disabled

Why not "array set state {0 disabled 1 disabled 2 disabled}" instead?

Show 5 quoted lines

>      }
>      if {$head eq $mainhead} {
> -	set state disabled
> +	set state(0) disabled
> +	set state(1) disabled

Similarly, "array set state {0 disabled 1 disabled}".

> +    }
> +    foreach i {0 1 2 3} {

Why do you go up to 3 when we never disable the 3rd entry?

Paul.