[PATCH 2/3] MainTab: fix missing update for 'amount_selected'

[Dirk Hohndel]

On Tue, 2013-12-10 at 01:11 +0200, Lubomir I. Ivanov wrote:
> On 10 December 2013 01:10, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
> > On 10 December 2013 01:07, Tomaz Canabrava <tcanabrava at kde.org> wrote:
> >> I think we should look in the code for every appearance of 'd->selected =
> >> true' and change it for void select_dive(int idx)
> >> sounds a bit safer.
> 
> but if you wan't me to use select_dive() instead i will modify the patch.

Let's look at a little more context here:


	if(editMode == ADD || editMode == MANUALLY_ADDED_DIVE){
		mainWindow()->dive_list()->unselectDives();
		struct dive *d = get_dive(dive_table.nr -1 );
		// HACK. this shouldn't be here. but apparently it's
		// how we can know what was the newly added dive.
		d->selected = true;
		sort_table(&dive_table);
		int i = 0;
		for_each_dive(i,d){
			if (d->selected) break;
		}
		editMode = NONE;
		mainWindow()->refreshDisplay();
		mainWindow()->dive_list()->selectDive( i, true );

So d->selected is abused here to mark the new dive. We can neither keep
the pointer nor the idx around to mark it - both can change as we sort
the table. So instead we call unselectDives() and mark the one dive that
we want to remember. And then we immediately afterwards walk the dive
table and then use the UI function to select that dive. So once
dive_list()->selectDive(i,true) has been called, everything should once
again be hooked up correctly.

Now the first thing I thought was "umm, maybe we should reset
d->selected to 0 here before calling selectDive(i,true), maybe that's
what's confusing things... but then I read the code for selectDive and I
wonder how this ever worked - as it DOESN'T keep our internal state and
the UI state in sync at all. And selectDives equally makes me squirm as
it sets the selected flags of the dives in newDiveSelection, but doesn't
keep amount_selected in sync, either.

Love it or hate it - parts of our code assume that amount_selected is
the number of dive structures in the dive_table where d->selected == 1

This code breaks that.

Maybe it's too early, maybe I need more coffee, but this really worries
me.

I'll stare at the code some more to see if I can figure out how to fix
it in an obviously correct way. But AFTER the final beta... not so good.
I hate to have to ask myself "how did this ever work"...


/D