[PATCH] Fix "Select tags" dialog behavior

Dirk Hohndel dirk at hohndel.org
Fri May 17 08:26:43 PDT 2013


On Fri, 2013-05-17 at 17:41 +0300, Sergey Starosek wrote:
> Dirk,
> 
> On Fri, May 17, 2013 at 5:14 PM, Dirk Hohndel <dirk at hohndel.org>
> wrote:
>         On Fri, 2013-05-17 at 13:05 +0300, Sergey Starosek wrote:
>         > * Apply filter only when OK button pressed
>         > * Restore tags selection from dive mask when Cancel button
>         pressed
>         > * Fix selection logic when selected or all dives are
>         filtered out (hide
>         >   profile, tooltips, etc.)
>         >
>         > Not sure whether call to repaint_dive() is required.
>         
>         
>         Great catch - thanks for fixing this.
>         I'd like to make a couple of small changes, though.
>         We have a helper function that deals with deselecting dives
>         correctly;
>         one thing that you are missing is that if you deselect the
>         selected_dive
>         then the selected_dive variable needs to change. You do this
>         if all
>         selected dives are hidden (amount_selected = 0), but not if
>         only some of
>         the selected dives (including the selected_dive) are hidden.
> 
> 
> Actually this piece of code was copied from preferences_dialog():
> 
> 
>          /* if we turned off displaying invalid dives. de-select all
>          * invalid dives that were selected before hiding them */
>         if (oldprefs.display_invalid_dives && !
> prefs.display_invalid_dives) {
>             for_each_dive(j, d)
>                 if (d->selected && d->dive_tags && DTAG_INVALID) {
>                     d->selected = 0;
>                     amount_selected--;
>                 }
>             if (amount_selected == 0)
>                 selected_dive = -1;
>         }
> 
> 
> Thus it may need to be reviewed.

Yes. And reviewing this and fixing things to work correctly pointed me
at yet another oddity in the selection code that I think I have now
fixed. But such a fundamental change this close before a release has me
VERY worried.

I'll spend a little more time thinking through my changes and will push
them out once I'm happy...

This will need serious testing.

/D




More information about the subsurface mailing list