Edition of cylinders broken

[Dirk Hohndel]

On Tue, 2014-01-07 at 10:01 +0100, Patrick Valsecchi wrote:
> Hi,
> 
> If you try to edit the O2 (for example) of an existing dive (loaded
> from the logbook, initially imported from a DC and inside no trip) and
> click the "save" button, the O2 is reverted back to the original
> value.
> 
> I tried to fix the problem, but I have trouble to understand the code:
>      1. Since I have no selected trips (just a single dive that is not
>         in a trip), maintab.c:458-460 is copying the dive into
>         multiEditEquipmentPlaceholder and cylindersModel->setDive is
>         called.
>              1. You can see in models.cpp:358 that we do no keep the
>                 pointer to multiEditEquipmentPlaceholder, but just
>                 keep the dive id.
>      2. When the O2 field is changed, we modify it in
>         cylinderAt(index), see models.cpp:200:
>              1. cylinderAt gets the dive
>                 from ::getDiveById(currentId), see models.cpp:162:
>                      1. getDiveById gets the dive from the global
>                         dive_table (not the
>                         multiEditEquipmentPlaceholder).
>      3. When I hit the save button, maintab.cpp:619 is executed and
>         resets the cylinder info to its original value, stored in the
>         un-modified multiEditEquipmentPlaceholder.

Great catch. My "don't store pointers" patch was too aggressive. This
one should have stayed a pointer as during the edit (until the moment
the save has been executed) the dive_table remains unchanged...

> Frankly, I don't know what this multiEditEquipmentPlaceholder is for,

It allows us to sanely edit multiple dives and do the right thing.

>  I don't understand why we don't store the dive in the CylindersModel
> instance instead of its ID,

That's the bug in my commits from earlier today.

>  I don't understand the logic of MainTab::acceptChanges. 

Yeah, that one has become a bit too complex for my liking. We use the
same code in a few different states and the acceptChanges function
doesn't do a good job of making this clear.

> That is too much unknown for me to come up with a fix without
> re-working the whole code into something I can understand.

I'll fix it (which is only fair since I broke it).

/D