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>
Commit 2cea115ddb "fixed" the selection
by hooking into mouseRelease events. An unintended consequence was
that scrolling with the cursor keys didn't update the current dive.
Therefore, also hook into the corresponding key-press events.
This is just horrible, but I'm not aware of any possibility to fix
it properly.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Fix two issues:
1) When narrowing the selection, we didn't get setSelection()
calls. Only, when the user released the mouse button was
the selection updated. Therefore, hook into the mouse-release-
event and update the UI if the selection changed.
2) We updated the ui in setSelection(). However, this was called
on mouse-move even if the actual selection didn't change.
Therefore, compare selection before and after processing of
the event and only refresh the UI if there are changes.
Clearly, this can only be a quick stopgap solution and we
should find out how to properly hook into the selection change
machinery. Though see commit 4928c4ae04
for the reason why we do things as we do them.
Fixes#2595
Reported-by: Willem Ferguson <willemferguson@zoology.up.ac.za>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
After selecting dives, the selected dive sites are collected.
This was done using the selectionModel()->selection().indexes(),
which is wrong, because it gives one index per row *and* column.
Accordingly, every dive site was added numerous times to the
array of dive sites to be selected. Change this to
selectionModel()->selectedRows(), which gives one entry per row.
Moreover, if multiple dives with the same site were selected,
this site was also added to the array multiple times. Therefore,
check the array before adding sites.
Note that all this should not change the user experience in
any way, it is only a code-hygiene thing.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The dive list accesses the filter model, therefore it makes sense
to also get the header data from there, even if they are only
forwarded from the source model.
This makes control flow more logical and will allow us to remove
the global DiveTripModel instance.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The selection changes upon completing the filter are handled by
the core. Don't do this explicitly in the DiveListView.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The QList served as backing store for backupExpandedRows()
and restoreExpandedRows(). However, these always came in
pairs in the same scope. There is no reason to store the
expanded rows over a longer time.
Therefore, return the expanded rows from backupExpandedRows()
and take them as argument in restoreExpandedRows(). Morover
replace the QList<int> by the much lighter std::vector<int>.
We certainly don't need copy-on-write, reference-counting and
immutability of iterators in this case.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Calls of these functions were removed in the previous commits.
Now, remove the functions themselves.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The old code saved, cleared and restored the selection. This
is not necessary anymore, because on model reset the selection,
which is stored in the core, is reset. Remove the unnecessary
selection handling.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveListView caught signals from the DiveTripModel
with the corresponding indexes. However, the DiveListView
is actually connected to the MultiFilterSortModel and
thus has to translate the indexes.
Instead, catch the signals in the MultiFilterSortModel,
transform them and resend. Let the DiveListView get
its signal from the MultiFilterSortModel.
Yes, this makes things less efficient because there is
an extra signal. On the upside, the makes data-flow much
more logical. Selection will have to be fixed anyway.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The goal here is to unify desktop and mobile by moving
selection code from the desktop-only view.
Currently, initialization of the selection still has to be
called from the view after connecting the appropriate signals.
This is due to the weird way in which create completely new
models when resetting them.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveListView has a function to select the first dive. Move
this to the core to be able to call it from all parts (not only
desktop) of the code.
Currently, this has a (small?) UI regression: when filtering dives
and no selected dive is visible anymore, the old code would select
the first dive in the list. The new code selects the newest dive,
which might not be the first if some sort-criterion is active.
To revert to the old behavior, it will be necessary to move the
sorting function likewise to the core.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since we now have a selection.c translation unit, put the selection-
related functions there.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When selecting all dives via CTRL-A or manually and the trips
were not expanded, the QSelectionModel sends a single
selectionChanged signal per trip. We are reloading the map
in every call, making this very slow.
I couldn't figure out how to make QSelectionModel behave more
nicely, therefore I chose the nuclear option: Remove the map
reloading from selectionChanged() and hook into all functions
that do selection changes. In these functions, first call the
original code and then do the selection-changed operations.
This will certainly need some tuning.
Reported-by: Willem Ferguson <willemferguson@zoology.up.ac.za>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The way this was accessed via Qt's model semantics was horrible.
This gives arguably more readable code, since we don't have to
shoehorn things through QVariants.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Split out the actual filtering from the MultiFilterSortModel.
Create a DiveFilter class that does the actual filtering.
Currently, mobile and desktop have their own version of this
class, though ultimately we may want to merge them.
The idea here is that the trip-model and undo-commands have
direct access to the filter-function and thus can take care
of keeping track of the number of shown dives, etc.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In the future we might want to use undo-commands for mobile as
well (even if not implementing undo).
Therefore, move the undo-command source from desktop-widgets
to their own commands top-level folder.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
To test whether an entry is a trip, we passed a pointer to the
trip through a QVariant and tested that for null-ity.
Passing pointers through QVariants has given us myriads of
problems in QML, therefore introduce a bool IS_TRIP_ROLE
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since requiring Qt >= 5.9.1, we can use the pointer-to-member-function
overloads of addAction (introduced in Qt 5.6). This has the advantage
of compile-time checking of the signal/slot parameters.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since changing the highlighting to use the selected dive, dive
sites with no dive were never highlighted in dive site mode.
Obviously, because there was no dive to be selected.
Therefore special-case all dive-site selection code to recognize
when we are in dive site mode.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Owing to the recent changes, when the selection flag in the
MapLocationModel was not updated correctly when the user
manually selected the dive. Do that before raising the
divesSelected signal in DiveListView::selectionChanged()
because that will cause the MainWindow to repaint the flags.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When clicking a dive site on the map, the QML code would set
the selected dive site, but then all dives of dive sites in
the vicinity were set. But still only the clicked-on dive site
was shown.
Therefore, don't set the list of selected dive sites in QML,
but later in DiveListView::selectDives(), where we know all
the dives that were selected.
This, again, gives nasty entanglement of diverse widgets and
models.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Move the declarations of the "report_error()" and "set_error_cb()"
functions and the "verbose" variable to errorhelper.h.
Thus, error-reporting translation units don't have to import the
big dive.h header file.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In 2e230da361 the dive-selection signals
were unified. Sadly, this was done in a suboptimal way resulting in
numerous calls to updateDiveInfo(), which refreshes the main-tab.
Firstly, the MainWindow connected to selection changes from both,
the undo-command and the divelist. Secondly, every selected dive
in the divelist caused a single signal.
Thus, connect only to the divelist (this is necessary for user-initiated
selection changes) and only send a single signal in the divelist
per selection-reset.
This is still less than perfect as updateDiveInfo() is called even
if the current dive doesn't change.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
For historic reasons, there where three distinct signals concerning
dive-selection from the undo-machinery:
1) divesSelected: sent newly selected dives
2) currentDiveChanged: sent if the current dive changed
3) selectionChanged: sent at the end of a command if either the selection
or the current dive changed
Since now the undo-commands do a full reset of the selection, merge these
three signals into a single signal.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Some commands tried to retain the current selection on undo/redo,
others set the selection to the modified dives.
The latter was introduced because it was easier in some cases, but
it is probably more user-friendly because the user gets feedback
on the change.
Therefore, unify to always select the affected dives on undo()/redo().
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
These functions were spread out over dive.c and divelist.c.
Move them into their own file to make all this a bit less monolithic.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
DiveListView::selectDives() would only select new dives but not clear
the old selection. Thus, callers would have to clear the selection
first. That would lead to two selection-changed signals.
Move the unselectDives() call into DiveListView::selectDives().
The DiveListView has an internal flag to prevent double signals.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
If fields in a trip are edited, select that trip, which will display
the trip in the notes-box.
This is realized by hooking into the tripChanged signal in the dive-list.
A layering-violation, perhaps?
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveTripModel was used to represent both, trip and list views.
Thus many functions had conditionals checking for the current mode
and both modes had to be represented by the same data structure.
Instead, split the model in two and derive them from a base class,
which implements common functions and defines an interface.
The model can be switched by a call to resetModel(), which invalidates
any pointer obtained by instance(). This is quite surprising
behavior. To handle it, straighten out the control flow:
DiveListView --> MultiFilterSortModel --> DiveTripModelBase
Before, DiveListView accessed DiveTripModelBase directly.
A goal of this commit is to enable usage of the same model by mobile
and desktop.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Currently, when selecting "Load media files even if time does not
match the dive time", the media are added to *all* selected dives.
Instead add it to the closest dive.
This seems like the less surprising behavior. Of course now if the
user really wants to add a media file to multiple dives, they will
have to do it manually.
To avoid a messy interface, this is solved by moving the iterate-
over-selected-dives loop to the core. Thus, a helper-function can
be made local to its translation unit.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
dive_create_picture() is called from DiveListView::matchImagesToDives()
with a copy of the picture-filename. But:
- On error the filename is not freed
- On success the filename is strdup()ed
Thus, in all cases the memory is lost. Instead, pass in a temporary
buffer using qPrintable().
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Just as we did for pointer to struct dive_site, make pointers to
struct dive and struct dive_trip "Qt metatypes". This means that
they can be passed through QVariants without taking a detour via
void *.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Traditionally, the DiveTripModel has its data sorted in opposite
direction to the core-data (chronologically descending vs. ascending).
This bring a number of subtle problems. For example, when filling
the model, trips are filled according to the *last* dive, whereas
later insertion points are according to the ->when value from the
core, which depends on the *first* dive.
As a start of fixing these subtleties, change the sort direction
to reflect the core-data. Ideally, this should lead to a removal
of the redundant data-representation.
Since the model is now sorted in ascending order, sorting has to
be enabled in the DiveListView constructor to reflect the
default-descending order.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since the QHeaderView of DiveListView is now the authority
over sort-column and sort-order, it makes little sense
to keep these as member variables. That would only risk
inconsistencies. Remove them and query the QHeaderView
instead.
We still need to keep track of currentLayout, as we
have to detect if it changes to change the underlying
model from tree to list or vice-versa.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveListView code had a very fundamental problem with its
header: Each had its own idea of who is responsible for sorting.
Since we can't easily change QHeaderView, accept QHeaderView
as the authority on sort-column and order.
To make this possible, split the reload() function in two
distinct functions:
- reload() reloads the model and sorts according to the
current sort criterion.
- setSortOrder() tells the header to display a certain
sort criterion. If this is a new criterion, it will then
emit a signal. In this signal, resort according to that
criterion.
Thus, the actual sorting code has to be moved from the
headerClicked() to a new sortIndicatorChanged() slot.
Morover, the sorting of the QHeaderView has to be used.
Reported-by: Stefan Fuchs <sfuchs@gmx.de>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Commit 6dc1d239f8 introduced a
well-defined sort order in the case of equal contents. It changed
the code for sorting by date to simply use the order of the
source model.
BUT: The source-model was already sorted in descending order
on date. Thus setting the default order on descening by date,
the data was then presented as *ascending* by date.
Change this back to descending by always using default-ascending
in the filter model.
Ultimately, the source model should simply reflect the ordering
of the core-data (ascending on date), but such a change is
too invasive shortly before release.
Reported-by: Jan Mulder <jlmulder@xs4all.nl>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
DiveListView::reload() was called for full reset of the dive list
and for changing the view (tree vs. lis) in DiveListView::headerClicked().
Since the latter does sorting by itself, a parameter "forceSort" was
introduced, which defaulted to true, but was set to false by
DiveListView::headerClicked().
To remove complexity, simply let DiveListView::headerClicked() set
the view by itself and remove tha parameter.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The QHeaderView::sectionPressed() signal was connected everytime
the list-view was reset. Likewise, setSectionsClickable() was
set to true everythime the list-view was reset.
Once in the constructor is enough.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The data-flow from C-core to list-view is as follows:
C-core --> DiveTripModel --> MultiSortFilterModel --> DiveListView
The control-flow, on the other hand, differs as DiveListView
accesses both MultiSortFilterModel and DiveTripModel, whereas
MultiSortFilterModel is mostly unaware of its source model.
This is in principle legitimate, as the MultiSortFilterModel might
be used for different sources. In our particular case, this is
not so. MultiSortFilterModel is written for a particular use case.
Therefore, model control-flow follow after data-flow: Let MultiSortFilterModel
set its own source model and DiveListView access the MultiSortFilterModel,
which then manages its source model.
This is not bike-shedding, but will enable a more flexible and
higher-performance sorting.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Remove three cases of rememberSelection() which did not possess
the corresponding restoreSelection() twins.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The selection was remembered/restored anytime the sort-order
changed. Yet, this is only necessary if the view (tree, list)
changes. Therefore, handle the selection only if this is the
case.
This automatically fixes the problem of the trip-selection
not being remembered if the view doesn't change. If the view
does change, trip selection is lost. But since the list view
doesn't have trips to start with, losing trip-selection seems
like an understandable behavior.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
On desktop, show the a sort indicator to give a visual feedback on changes
of the sort order. This is trivially done by calling the
setSortIndicatorShown() function in DiveListView's constructor.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
