From: Pratyush Yadav <me@yadavpratyush.com>
Date: Tue, 03 Sep 2019 21:49:42 GMT
Subject: Re: [PATCH v4] git-gui: Add hotkeys to set widget focus
Message-ID: <20190903214942.ubcbdgtphpwnaxs6@yadavpratyush.com>
In-Reply-To: <20190903180704.11812-1-birger.sp@gmail.com>

Thanks for the re-roll. Before you send out the v5, a couple more things 
you missed. Other than those, LGTM.

On 03/09/19 08:07PM, Birger Skogeng Pedersen wrote:
[snip]
> @@ -2640,6 +2642,27 @@ proc show_less_context {} {
>  	}
>  }
>  
> +proc select_path_in {widget} {

There was a suggestion from me and Bert about changing the function 
name.  I suggested "focus_widget", and Bert suggested 
"focus_and_select_path_in".  Choose whichever you feel is better.

> +	global file_lists last_clicked selected_paths
> +	global file_lists_last_clicked
> +
> +	set _list_length [llength $file_lists($widget)]
> +	if {$_list_length > 0} {

Since we only use $_list_length in one place after my suggestion, why 
not just remove it entirely?

That makes your if statement:

if {[llength $file_lists($widget)] > 0} {
	...

[snip]

-- 
Regards,
Pratyush Yadav