The signals/slot names for dive selection changes were a mess.
Unify on divesSelected(). Firstly, selectionChanged() is a Qt
thing. Secondly, it is consistent with tripSelected().
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The trip selection code was an awkward layering violation.
Whereas dive selections due to dive undo-commands trickled
down via DiveTripModel-->MultiFilterSortModel-->DiveListView,
for trip editing, the DiveListView directly intercepted the
TripEdited signal.
Instead, mimic the dive-selection code. This is a bit longer
but more consistent and logical. The undo/redo of trip changes
is now also a "programmatical" change of the selection.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
After sending a selection-change signal, there follows a current
dive changed signal. Combine these two into a single signal, since
usually the current dive is changed when the selection is changed.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This tries to encapsulate the management of the current dive and
divecomputer in the selection code. The current dive is alreay
set by setSelection(). Add a new parameter to also set the
current divecomputer. If -1 is passed, then the current
computer number is remained. This will allow us to audit the code.
Because for now, the whole "current dive computer" thing seems
to be ill-defined.
This fixes a bug: the dive-computer number wasn't validated
when making a new dive the current dive. The new code has some
drawbacks though: when selecting a whole trip, the validation
will be called for all dives in the trip and thus the dive computer
number will depend on the dive with the lowest amount of dive
computers in the trip. This will need to be fixed.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The parser API was very annoying, as a number of tables
to-be-filled were passed in as pointers. The goal of this
commit is to collect all these tables in a single struct.
This should make it (more or less) clear what is actually
written into the divelog files.
Moreover, it should now be rather easy to search for
instances, where the global logfile is accessed (and it
turns out that there are many!).
The divelog struct does not contain the tables as substructs,
but only collects pointers. The idea is that the "divelog.h"
file can be included without all the other files describing
the numerous tables.
To make it easier to use from C++ parts of the code, the
struct implements a constructor and a destructor. Sadly,
we can't use smart pointers, since the pointers are accessed
from C code. Therfore the constructor and destructor are
quite complex.
The whole commit is large, but was mostly an automatic
conversion.
One oddity of note: the divelog structure also contains
the "autogroup" flag, since that is saved in the divelog.
This actually fixes a bug: Before, when importing dives
from a different log, the autogroup flag was overwritten.
This was probably not intended and does not happen anymore.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since the only caller was C++ code, this can be done in
C++ code, which removes memory-management headaches.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Search the index of an item in a container. Compare by
equality or a lambda. The lack of these have annoyed me for a
long time. Return the index of the first found element or
-1 if no element found.
Currently, only supports random-access operators. Might be
trivially changed for forward iterators.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
There were a number of classical "for (i = 0; i < size; ++i)"
loops. Replace them either by plain range based loops if the index
is not used or by enumerated_range() loops.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The moveInVector() function was defined in qthelper.h, even
though it has nothing to do with Qt. Therefore, move it into
its own header.
Morover, since it is a very low-level function, use snake_case.
And rename it to move_in_range(), because it does not only
work on vectors, but any range with random-access iterators.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The formatting of the tooltips describing the columns
was repeated in two functions. Infuriatingly, there were
to minor differences: "Max. CNS" vs. "Max CNS" and
one version understood "Country".
Break that out into a single function to avoid these
inconsistencies and to save a few lines of code.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In general, replace "dive master" by "dive guide".
However, do not change written dive logs for now. On reading,
accept both versions.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
To enable grouping by trip in the statistics module, split
the get_trip_title() function in a version that appends
a "(n dive(s)" string an one that doesn't. The statistics
module doesn't want that added string, since it displays
the number of dives in a different way.
Also, move the functions to string-format.h, where these
are collected. And rename them to camelCase. Yes, it's
ugly, but consistent with most other C++ code in the code
base.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The dive selection was initialized during data-reset. However,
this emitted a signal before all data-reset routines were run.
Ultimately, this led to access-after-free in the statistics code.
Instead, move the select_newest_visible_dive() signal from the
divelist-model to the process_loaded_dives() function. There
is no point in initializing the selection if the dive data
is cleared after all.
This change broke closing of the log, because the UI-selection
was not reset. Therefore, when clearing the data, clear the
selection before proceeding with clearing.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The icons shown in the dive list were rendered for every single
access. Render them only once. This supposes that the
defaultIconMetrics structure does not change once the icons are
rendered!
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The shown_dives variable was reset by the dive_list code. Arguably,
the filter should keep track of the number of shown dives, so move
the resetting there. This means adding a new "reset()" member function
to the filter and call that instead of "updateAll()" when the core
data is reset.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When editing a dive, a DiveObjectHelper of the unmodified dive
was created to compare the edited with the old values. Since
the DiveObjectHelper is used here only as a pointless wrapper
around the formatting functions, call these functions directly.
However, note that the code is in principle wrong since the
change to the mobile-models, which do not use the DiveObjectHelper.
The real fix would be to reload the data from the model to prevent
going out-of-sync with respect to the formatting routines!
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
At this point (post grantlee), DiveObjectHelper is just pointless
glue code. Let's remove it from the printing code and call the
formatting functions directly. If necessary, move these functions
to core/string-format.cpp.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The mobile version of the list used string formatting functions
defined in DiveObjectHelper and declared in mobilelistmodels.h.
Very confusing. Move them to a separate source file where - in
the long run - all the string-formatting functions, which
are scattered all over the place, can be collected.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Technically get_dive(i) could return a nullptr. But given the range for i that
can never happen. Still, the test is extremely cheap and doesn't hurt.
Fixes CID 354768
Fixes CID 354766
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When moving dives between trips, the core moves the dives internally
and sends a signal to the model. The model adds and removes the dives
accordingly. However, when adding the new dive, the old trip hasn't
changed its position, so the ordering is wrong leading to an inconsistent
state.
Therefore, remove the dives first and then readd them. There could
still be pathological cases where this fails. However, in the short
term this is an improvement. Note that in similar cases, the dives were
indeed removed then added, so this case here seems to be an oversight.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The get_trip_date_string() formatted, as the name implies, the date
of a trip. It was passed a number of parameters and had only one
caller, which would also add the location if it existed.
Therefore, move all that logic into the helper function and
name it get_trip_string().
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
If the dive timestamp changes, the dive could move in the dive list. But the
current dive actually doesn't change (it's still the same dive, right?). Yet
we need to update the dive list as well as the shown dive (especially if this
is after adding a dive, which is first inserted with the current time and then
updated with whatever the user enters).
Fixes: #2971
Suggested-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
At some time, when introducing the global reset signal the filter
stopped being reloaded when loading a new log. This leads to very
strange UI behavior: dives disappear when editing fields unrelated
to the filter.
Therefore, when reloading the model, reset the filter. One might
argue whether this is the correct place. On the other hand, we
might even make the filter a sub-object of the dive-list model.
Let's think about this.
Partially solves #2961
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Move this function from maintab.cpp to qthelper.cpp. Since the
functionality was used in numerous places, use the helper function
there as well. This removes a number of inconsistencies. For example,
sometime setTimeSpec(Qt::UTC) was called, even though the
QDateTime object was already created with that time spec.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We already allow filtering by tags, but don't even show them on mobile.
That seems rather inconsistent.
First step is to make the tags available to the QML layer.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
To reset the core data structures, the mobile and desktop UIs
were calling into the dive-list models, which then reset the
core data structures, themselves and the unrelated
locationinformation model. The UI code then reset various other
things, such as the TankInformation model or the map. . This was
unsatisfying from a control-flow perspective, as the models should
display the core data, not act on it. Moreover, this meant lots
of intricate intermodule-dependencies.
Thus, straighten up the control flow: give the C core the
possibility to send a "all data reset" event. And do that
in those functions that reset the core data structures.
Let each module react to this event by itself. This removes
inter-module dependencies. For example, the MainWindow now
doesn't have to reset the TankInfoModel or the MapWidget.
Then, to reset the core data structures, let the UI code
simply directly call the respective core functions.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We show an icon whether there are pictures and whether they are
before or after the dive. Thus, the list models must emit the
proper signals when the pictures of a dive change.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
For consistency with equipment, use our table macros for pictures.
Generally tables (arrays) are preferred over linked lists, because
they allow random access.
This is mostly copy & paste of the equipment code.
Sadly, our table macros are quite messy and need some revamping.
Therefore, the resulting code is likewise somewhat messy.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
To display changed SAC values it is necessary that the models
emit changed signals when cylinders are edited. An alternative
might be that the undo commands emit dive-changed signals themselves.
Fixes#2814.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In 9310d72943, resetting of the dive
sites was moved to DiveTripModelBase::reset(). This seemed like a
good idea, because it means that the location list is reloaded
every time the dive list is reset.
Unfortunately that function is only used on mobile, thus on desktop
the dive site model is not updated. Do that in
MultiFilterSortModel::resetModel(), because this is always called
when the dive list is reset. Desktop differs from mobile in that
two different models are used depending on whether we are in list
or in tree mode.
Fixes#2749
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When initializing the fulltext-cache and the dive-list, every
100 dives a notification was shown. I had a feeling that this
made startup significantly slower, but that could have been
purely psychological.
Therefore I measured and indeed, removing the fine-grained
notification, it becomes *significantly* faster. For a 3500
dives test log with mobile-on-desktop:
Initialization of the fulltext: 1350 ms -> 730 ms (-46%)
Initialization of the divelistmodel: 689 ms -> 113 ms (-83%)
Let's remove the fine-grained notification. There *is* a visual
indication of work-in-progress anyway.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
To mark invalid dives, we use a struck-out font, which was a static
variable at translation unit scope, i.e. initialized at application
startup. Sadly, this crashes on iOS.
It is unclear when we can initialize fonts. Try to move initialization
to the constructore of DiveTripModelBase and make the font a member
of that class. For consistency, also make the invalidBrush a member
of this class.
This now means that the diveData function cannot be static anymore,
since it needs access to the font and brush. But OK.
Reported-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Mark invalid dives in the dive list by striking them out
and rendering them with a grey color. The color-change is
not sufficient, because the default model delegate ignores
color hints if the item is selected.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When the dive list is cleared or updated, the entries in the
divesite-model become stale and therefore the divesite-model
(with the actual name LocationInformationModel) also must be
updated. This was done manually in some parts of the code and
forgotten in others. Therefore, do it directly in the clear()
and reset() function of the dive list-model.
This might be a bit of a layering violation: why should one
model call into another if they are not in parent/child
relationship? However, this seems easier than introducing
a global "reset dives" function that coordinates the models.
Moreover, it does not appear 100% safe: if the clearing of
the divesite model causes accesses to the divelist-model,
they happen in the midst of a model reset and we had horrible
bugs with that kind of things. However, I don't think that
should happen.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The setData() function was used to edit the number of a dive.
However, that doesn't appear to be functional. Therefore, remove
the code. There is a context-menu entry "renumber dives" for that.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The diveListNotifier.divesSelected() signal is used to inform the
models of a selection change. It sent the current dive as a second
parameter. This is redundant, because the only sender of the signal
sets current_dive just before sending the signal. Remove the
parameter, which appears to be an artifact.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveTripModelTree::divesSelected() returned early if
no dives were selected. Thus, the current dive was not
updated. Remove the check for no dives.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Grammar-nazi ran
git grep -l 'indexes' | xargs sed -i '' -e 's/indexes/indices/g'
to prevent future wincing when reading the source code.
Unfortunatly, Qt itself is infected as in
QModelIndexList QItemSelection::indexes() const
Signed-off-by: Robert C. Helling <helling@atdotde.de>
This is only used in the mobile UI where the sort direction is fixed and we
refer to dives based on the tree model. So the terms used and the concepts
that these rely on should be guaranteed to be valid.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When the dive data is cleared, all pointers in the undo-stack become stale.
Desktop explicitly called Command::clear() in that case, but mobile doesn't.
Thus, move the clear() call into DiveTripModelBase::clear()
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
