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

Hi,

On Sat, Jan 7, 2012 at 20:43, Bert Wesarg [off-list ref] wrote:

quoted

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.