[PULL REQUEST] Delete multiple dives

[Lubomir I. Ivanov]

On 27 September 2012 23:05, Dirk Hohndel <dirk at hohndel.org> wrote:
> 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.
>

my first thoughts were in that direction (e.g. an "expanded" flag in
dive_trip struct), but since i don't know
the code well enough, i've decided on this direction, which is more
complicated and less efficient (way to many iterations). :\

>> 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.
>

not sure why TBH. in the example XML it always crashes at idx = 7 (or was it 8).
also i've added a check if the selection iterator is not a trip in
tree_selected_foreach(). since this was also causing a crash -
if one select dives and trips and then right clicks a dive for "delete dives".

>> 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.
>

just realized the current master actually returns some Gtk-CRITICAL
and crashes which some of these patches solve.
e.g. right click (no selection) on the first dive in a trip and "delete dive".
i think i've caused this with delete_single_dive() in
delete_dive_cb(), but i've tested this multiple times for the previous
set of patches. it could be the XML somehow.

>> 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.
>

it will look much nicer, i think.

lubomir
--