[PATCH] Refrain from automatic selection and trip expansion in the divelist

Lubomir I. Ivanov neolit123 at gmail.com
Thu Sep 20 14:33:02 PDT 2012


On 21 September 2012 00:24, Dirk Hohndel <dirk at hohndel.org> wrote:
> "Lubomir I. Ivanov" <neolit123 at gmail.com> writes:
>
>> From: "Lubomir I. Ivanov" <neolit123 at gmail.com>
>>
>> Divelist fill:
>> When we first build the list, leave it to the user to first interact with it
>> - expand trips, select dives. An example scenario is that when
>> the topmost trip is automatically opened and the first dive inside selected,
>> while the trip itself contains a lot of dives, the user may be looking for
>> a dive inside the bottommost trip, therefore has to scroll down. This is
>> in the lines of user preference logic and potentially there could be
>> application settings for it.
>
> I disagree with this change. I don't want subsurface to open with just a
> divelist and no dive displayed. That seems simply unattractive to me.
> And displaying the last dive in the list seems a reasonable starting
> point.
>

that's a good point actually. i've ignored the fact that the profile
will be empty. :\

>> Delete:
>> When deleting a dive, similar to some GUI file managers, leave no dives
>> selected afterwards. Sill expand the last trip which we can obtain from
>> the actual callback's GtkTreePath.
>
> Sorry, I disagree with this one as well. If I have multiple dives
> selected and delete one of them, the others should stay selected.
>

actually, there may be a small bug here or something strange.
on current master -  7e8a4760100ae091a569:
when multiple dives are selected (say 3) and one of them gets deleted
the remaining length remains 3 where the first dive on top gets
selected instead.
will post this one in the other thread as well...

> Worse, your patch gets "amount_selected" into an inconsistent state (it
> will likely be too big) which can trigger all kinds of problems. For
> example, if you deleted the very last dive in a data file and then try
> to add a new dive, with your patch subsurface will crash as it tries to
> look at the selected dive that it thinks still exists even though the
> data structures have been freed.
>

i caught the amount_selected = 0 part, but forgot about it TBH...

> So NACK on both patches from me.
>

oh well... :-)

lubomir
--


More information about the subsurface mailing list