Re: [PATCH] git-gui: fix selection regression introduced in a8ca786991

From: Bert Wesarg <bert.wesarg@googlemail.com>
Date: Jan 9, 2012, 09:28 UTC
Message-ID: <CAKPyHN0xRD7qYyLYaCB9u7mhCZYFObuTdJGHq-rRST-cEhTtXA@mail.gmail.com>
In-Reply-To: <14628854a651ab0202e3f82be9b245331cf9029a.1325965254.git.bert.wesarg@googlemail.com>

Hi,

On Sat, Jan 7, 2012 at 20:43, Bert Wesarg <bert.wesarg@googlemail.com> wrote:

Show 12 quoted lines

> While fixing the problem from a8ca786991, it introduces a regression
> regarding what happen after the multi selected file operation (ie.
> one of Ctrl-{T,U,J}) because the next selected file could not be handled
> by such a subsequent file operation.
>
> The right way is to move the fix from this commit down into the show_diff
> function. So that all code path add the current diff path to the list of
> selections.
>
> This also simplifies helper functions for these operatione which needed
> to handle the case whether there is only the current diff path or also
> a selction.

I think we need to think this more through, especially with input from Shawn, please.

I have now find out, that git-gui has two selections in the file lists. The first is that for the current path for what we show the diff (the tag for this is called 'in_diff') and the the second is that for the current list of paths which are selected ('in_sel'). The file list operations 'staging', 'reverting', 'unstaging', work either on 'in_sel'; if that is not empty, or on 'in_diff'. The problem I've now realized is, that these two selections share the same visual hints, ie. a lightgray background.

The problem I tried to solve in a8ca786991 was, that adding paths to the selection with Ctrl-Button-1 or Shift-cutton-1, didn't included the current diff path in the subsequent file list operation. But I would have expected it, because it was visual in the 'selection'.

My current 'workaround' is to make the two selections visually distinguishable (and reverting a8ca786991), by using a different background color for the 'in_sel' tag and also the italic font, so that it is still possible to see whether the current diff path is in the selection or not:

@@ -717,11 +717,11 @@ proc tk_optionMenu {w varName args} {
 proc rmsel_tag {text} {
 	$text tag conf sel \
 		-background [$text cget -background] \
 		-foreground [$text cget -foreground] \
 		-borderwidth 0
-	$text tag conf in_sel -background lightgray
+	$text tag conf in_sel -background SlateGray1 -font font_diffitalic
 	bind $text <Motion> break
 	return $text
 }

 wm withdraw .
@@ -3557,11 +3557,11 @@ if {$use_ttk} {
 	.vpane.files add .vpane.files.index -sticky news
 }

 foreach i [list $ui_index $ui_workdir] {
 	rmsel_tag $i
-	$i tag conf in_diff -background [$i tag cget in_sel -background]
+	$i tag conf in_diff -background lightgray
 }
 unset i

 set files_ctxm .vpane.files.ctxm
 menu $files_ctxm -tearoff 0

I'm not very pleased with this, but at least it is now possible to
visual recognize what files will be handled by a subsequent file list
operation.

Any input is more than welcome.

Regards,
Bert