[PULL REQUEST] Delete multiple dives

[Dirk Hohndel]

On Sep 27, 2012, at 12:48 PM, Lubomir I. Ivanov wrote:

> On 26 September 2012 20:44, Lubomir I. Ivanov <neolit123 at gmail.com> wrote:
>> On 26 September 2012 18:14, Dirk Hohndel <dirk at hohndel.org> wrote:
>>> Could you add code that remembers which trips were expanded and
>>> re-expand those trips that still exist after the delete continues?
>>> Extreme example: click 'expand all', delete a dive and see everything
>>> collapse again. Unexpected...
>>> 
>>> Thanks
>>> 
>> 
>> np,
>> will take a look at this tomorrow morning (GMT+3).
> 
> sorry for the delay,
> here is what i came up, but not sure if this is the best solution.

Reading through the code it seems quite intricate. I need to think about this
some more before applying the patch. There has got to be an easier way to
do this.
For example, we could add a flag to dives that are in a trip to state whether
their trip is expanded (so add this to the struct dive). This way all you do is
walk the dive_table and can restore the expanded state. And with the collapse / 
expand callbacks it should be easy enough to keep this in sync.

> i've noticed a strange problem and perhaps you could take a look at that:
> when i switch the list to different sorting e.g. "by rating" the trips
> disappear (i'm sure this is expected). but when i select all dives
> (see attachment XML) and then click  "delete dives", some of them are
> left out. this is mainly because i have a check if get_dive() returns
> NULL in delete_single_dive(). as far as i understand, it shouldn't
> return NULL. the same problem is absent when deleting all dives with
> sorting "by date" enabled.

get_dive would return NULL if no dive for that index exists. So having that
happen on elements of the tree is rather worrisome as it means that something
went out of sync.

> another problem is that the trips' "when" flags are not updated if we
> delete dives when viewing dives sorted "by rating" and then returning
> back to the "by date" sorting. i don't think update_trip_timestamp()
> is optimal for this, since it takes an iterator for a parent (trip
> iterator) that does not exist when we are not sorting "by date".

The trick is to use TREEMODEL(dive_list) and looking up the dive_trip that
way. So you get your trip from dive->divetrip and then you can get the tree
done that matches this by using the TREEMODEL. This should all work,
regardless which model is actually displayed.

I thought I had this (and the "must delete trip" case below) handled for 
the delete single dive case when I implemented that originally, but I'm not 
longer 100% sure of that now.

> a question here i guess is: should the trips ever disappear if we sort
> by something other than date ?

Well, we should certainly try to keep things consistent, regardless of sort order.

Having empty trips stick around certainly sounds like an ugly corner case that 
I would love to get rid of.

/D