After calling dive_list_update_dives() in delete_selected_dives_cb(),
if the selection length is zero, we can clear the display widgets
not to show information of a deleted dive.
[Dirk Hohndel: please watch your whitespace - you once again added a bunch
of empty lines that really didn't help the code...
I removed them]
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The existing code (both my first single dive delete and then Lubomir's
multi dive delete code) had way too many issues and was just painfully
inefficient.
This new code takes a radically different approach and mostly ignores the
Gtk tree model (as that gets recreated after a delete, anyway) and instead
is linear time on the number of dives in the list. It does do its best to
maintain the existing selection and the expand state of tree model (the
latter isn't possible if we have switched to the list model).
Many thanks to "Lubomir I. Ivanov" <neolit123@gmail.com> for his work on
this - this commit actually contains a few lines out of one of the patches
that he wrote.
Reported-by: "Lubomir I. Ivanov" <neolit123@gmail.com>
Tested-by: "Lubomir I. Ivanov" <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Both gtk_tree_selection_selected_foreach() and
gtk_tree_selection_get_selected_rows() are problematic.
gtk_tree_selection_get_selected_rows is not compatible with older GTK,
while gtk_tree_selection_selected_foreach() should not be used to
modify the tree. A workaround to is allocate memory and store what
is returned from the gtk_tree_selection_selected_foreach() callback
function as a GtkTreeIter array. Once done iterate trought the array
and pass the values to delete_single_dive().
A bit excesive, but it is not certain how safe is modifying the tree
while in the "_foreach" loop, even if it only shows a warning.
On the other hand the GTK source shows gtk_tree_selection_get_selected_rows()
to be a rather complicated and slow method.
Inside delete_single_dive(), once a dive is no longer part of "dive_table"
and if the dive was part of a trip, remove the dive from the tree
(gtk_tree_store_remove()) and call update_trip_timestamp().
The struct type "tree_selected_st" and tree_selected_foreach() are
reusable.
Reported-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Moved portion of the code from delete_dive_cb() to a function called
delete_single_dive(), that directly accepts a GtkTreeIter pointer.
Added the function delete_selected_dives_cb(), which is called
when calling "Delete dives" from the combo box for the selected dives.
The above function iterates trought the selection calling
delete_selected_foreach(), which on its own calls delete_single_dive().
The "for-each" API in this case looks much prettier C code wise,
however we do potentially create an extra jump and also
do not have anything but the redirection:
delete_selected_foreach() -> delete_single_dive()
Probably slighly slower than using gtk_tree_selection_get_selected_rows(),
performance wise, but less C code.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
When clicking multiple dives in the list, check if more than one
are selected and if so show the text "Delete dives".
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
After deleting a dive the dive list is recreated. If there are still dives
selected we should select the last dive as well. If there isn't any dive
selected, then the last dive is as good a default as any, I guess.
Reported-by: "Lubomir I. Ivanov" <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Merge the dive trip rewrite by Dirk Hohndel.
This just merges the dive trip changes with the timestamp handling
changes. There were multiple small data conflicts, along with some
newly added 'time_t' cases in the dive trip handling that needed to be
converted to 'timestamp_t' along the way.
* 'divetrip-rewrite' of git://github.com/torvalds/subsurface:
Convert FIND_TRIP into function
Partial rewrite of the dive trip code
Check if trip is NULL before calling DIVE_TRIP
The pointer size may not be large enough to contain a timestamp, so make
FIND_TRIP() just pass the pointer to the timestamp instead.
And use an inline function instead of macros with casts. That gets us
proper type safety while at it, so that we get a warning if somebody
doesn't pass the expected "timestamp_t *". Plus the code actually looks
simpler and way more straightforward.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This helps us deal with the issue that the g_list convenience functions
don't allow us to easily compare 64bit values on 32bit architectures. And
since these convenience functions are truly trivial in nature, it seemed
easier to simply implement our own logic here.
In the process I moved all the dive_trip_list helper functions into the
same spot in divelist.c
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is the same bugfix that Lubomir did in the master branch, but now
on top of the new 64-bit timestamp_t model. So now we also remove the
comment about the year 2038 problem, because it's not true any more. We
do all the date handling in a 64-bit integer.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This introduces a new data structure for dive trips - reuseing the struct
dive just got way too messy.
The dive_trip_t datastructure now allows the code to remember if the trip
was auto generated or if its time stamp changed when dives where added to
the trip during auto generation.
The algorithm also distinguishes between dives that were intentionally
added to a trip (either in an XML file or by adding them to trip in the
UI) and dives that were added to trips via autogen. Saving dives that were
added to trips via autogen makes that assignment "intentional".
With this partial rewrite several of the oddities of the old code should
be resolved - especially turning autogen on and off again should get the
divelist back to the previous stage.
Also, when dives are merged during file open or import we now try to pick
the correct tripflag (instead of just ignoring the tripflag completely and
resetting it to TF_NONE by mistake).
Finally, the dive trip debugging code got more verbose and is trying
harder to detect issues at the earliest time possible.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This makes the time type unambiguous, and we can use G_TYPE_INT64 for it
in the divelist too.
It also implements a portable (and thread-safe) "utc_mkdate()" function
that acts kind of like gmtime_r(), but using the 64-bit timestamp_t. It
matches our original "utc_mktime()".
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Other places have this check, but for this particular one a crash
can be reproduced:
./subsurface ./dives/*
log -> autogroup
log -> autogroup
Against d14932058f
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Thanks to Christian for running the static code analysis tool against
subsurface...
There were some false positives, a few style issues that I'll ignore for
now, and two actual potential bugs.
First: Don't check unsigned variables for < 0
This has been around for a while and we are lucky that while technically a
bug it still works as expected. Passing a negative idx simply turns it
into a very large unsigned integer which then fails the > dive_table.nr
test. So it still gets a NULL returned. A bug? Yes. Critical? No.
Mismatched allocation and free
This is an actual bug that potentially could cause issues. We allocate
memory with malloc and free it with g_free. Not good.
Reported-by: Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn@axis.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This could cause a crash if deleting the last dive and manually adding a
new one.
Reported-by: Henrik Brautaset Aronsen <subsurface@henrik.synth.no>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This fixes a bug that Lubomir reported in a different way from the patch
that he providede; I believe this to be more generic.
Reported-by: "Lubomir I. Ivanov" <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is just fixing an embarrassing oversight. Now we should prompt the
user about saving the file whenever something changes.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
divelist.c:
Replaced "gtk_tree_path_get_indices_with_depth()" with the coupled alternative:
int depth = gtk_tree_path_get_depth(path);
int *indices = gtk_tree_path_get_indices(path);
for compatibility GTK+ < 2.22
*:
Replaced all usage of "cairo_rectangle_int_t" with "cairo_rectangle_t"
for compatibility with Cairo < 1.10.
Both modification make building Subsurface possible on a fairly recent Debian
distribution, which reports to have the version of the abovementioned
libraries "up-to-date", yet they are slightly outdated.
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When first trying to deal with this I opted to go with a two pass appoach
which seemed easy as it used existing infrastructure, but turned out to
run into a couple of odd corner cases that would have been really ugly to
deal with.
So I threw this code away and am instead doing this in a single pass,
carefully checking as we go if there is an appropriate trip we can use.
To me the new code is much easier to read and seems much cleaner.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This makes things more consistent with the merge with trip above option -
if multiple dives are selected then the consecutive set of selected top
level dives below the dive on which a user right-clicked are all added to
the newly created trip.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
There is an interesting issue when adding new dives into a dive list with
existing trips. Since fill_dive_list walks the list backwards and trips
are determined by the timestamp of the first dive in a trip, it is
non-trivial to know when a dive is added if it should be part of an
existing trip or not. Let's say when we see the dive we can also see a
trip entry that starts four days earlier. Without looking forward in the
list of dives we cannot tell if this is a multi-day trip that this dive
would fit into, or if there is a break of more than tree days (our current
trip threshold).
Instead this commit adds a second scan of the dives in chronological order
that does the right thing for new dives.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Yet another trip manipulation function. The dive we are on (or that dive
and the selected dives below it) are merged into the trip directly above.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When inserting a trip into the dive_trip_list we already check for
duplicate trips, but we still kept the additional dive_trip around.
With this change we instead replace it with the existing one.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
There are a few obvious trip manipulations on multiple dives that haven't
been implemented, yet. This commit handles the case when we have multiple
dives selected and right click on one of them. It now removes all of those
dives from their trips (instead of just the one that we clicked on).
Still todo is the inverse operation. Select a group of consecutive dives
and turn them into a trip.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Miika suggested this - we should be able to merge with the trip below and
not just the trip above (oh, and call them "above/below" instead of
"previous").
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Instead of using our generic helper function the code in
remove_from_trip_cb tried to implement the special case - and got it
wrong. This fixes yet another crash that Henrik found.
Reported-by: Henrik Brautaset Aronsen <subsurface@henrik.synth.no>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The update_trip_timestamp function can indeed get called with no children
present, just before that trip is then removed. So instead of adding
complicated special cases, this just bails out of the function.
Reported-by: Henrik Brautaset Aronsen <subsurface@henrik.synth.no>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Cut and paste error when creating this function.
Reported-by: Henrik Brautaset Aronsen <subsurface@henrik.synth.no>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This adds the ability to auto create trips from the menu. It's a toggle
entry (and while at it, we made the zoom toggle a toggle entry as well).
We can therfore switch back and forth between auto generated trips.
There is one bug. Assume you have no trips. You manually create a trip
from some dives out of a group of trips that autogen would turn into a
trip. Now you turn on autogen and this trip gets expanded with all the
dives that would normally be grouped together. If you turn off autogen
again, all those dives are still part of the remaining (initially manually
created) trip. Working around this issue seemed a lot more work than the
likelihood of anyone running into it seemed worth.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We were using the tree model to check the selection, even though the
active model is the list model after switching to a different sort column.
To make things clearer I renamed the access macros to be more consistent.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Now that we can load and store trips we needed to add the capability to
manipulate those trips as well.
This commit allows us remove a dive from a trip via a right click
operation on the dive list.
The commit also adds code to split a trip into two, to merge two trips and
to create a new trip out of a top level dive.
To make all that useful this commit changes the right-click on the dive
list to identify and act on the record we are actually on (instead of
acting on the selection).
The right-click menu ("context menu") changes depending which divelist
entry the mouse pointer is on - so different operations are offered,
depending on where you are.
We also add simplistic editing of location and notes for a trip (but the
notes are never displayed so far).
To make our lives easier this commit adds a link from the dive to the dive
trip it is part of. This allowed to hugely simplify the auto trip
generation algorithm (among other things). The downside of this change is
that there are now three different ways in which we express the
relationship of dives and trips: in the dive_trip_list, in the tree_model,
and with these pointers. Somehow this screams that I should rethink my
data structures...
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
In preparation for the next stage of the trips handling this commit makes
the macros used to access trips (and some frequently used variables for
the tree and list models) more consistent.
This also changes the way we display un-grouped dives in the dive list,
i.e. dives that are not part of a dive trip. Their dive number is now
printed bold.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The tree_storage only provided enough space for an int for DIVE_DATE. But
at least on 64bit Linux, an int is 32bit yet a time_t is 64bit. Until 2038
this only causes issues in some odd situations, after 2038 this would be
an obvious bug.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Merge the initial 'track trips explicitly' code from Dirk Hohndel.
Fix up trivial conflicts in save-xml.c due to the new 'is_attribute'
flag.
* 'trips' of git://git.hohndel.org/subsurface:
Fix an issue with trips that have dives from multiple input files
Some simple test dives for the trips code
First cut of explicit trip tracking
The existing code didn't handle the case of different trips for the same
date coming from different sources. It also got confused if the first dive
processed (which is, chronologically, the last dive) happened to be a
"NOTRIP" dive.
This commit adds a bit of debugging infrastructure for the trip handling,
too.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This code establishes the explicit trip data structures and loads and
saves them in the XML data. No attempts are made to edit / modify the
trips, yet.
Loading XML files without trip data creates the trips based on timing as
before. Saving out the same, unmodified data will create 'trip' entries in
the XML file with a 'number' that reflects the number of dives in that
trip. The trip tag also stores the beginning time of the first dive in the
trip and the location of the trip (which we display in the summary entries
in the UI).
The logic allows for dives that aren't part of a dive trip. All other
dives simply belong to the "previous" dive trip - i.e. the dive trip with
the latest start time that is earlier or equal to the start time of this
dive.
This logic significantly simplifies the tracking of trips compared to
other approaches that I have tried.
The automatic grouping into trips now is an option that defaults to off
(as it makes changes to the XML file - and people who don't want this
feature shouldn't have trips added to their XML files that they then need
to manually remove).
For now you have to select this option, then exit the program and start it
again. Still to do is to trigger the trip generation at run time.
We also need a way to mark dives as not part of trips and to allow options
to combine trips, split trips, edit trip location data, etc.
The code has only had some limited testing when opening multiple files.
The code is known to fail if a location name contains unquoted special
characters like an "'".
This commit also fixes a visual inconsistency in the preferences dialog
where the font selector button didn't have a frame around it that told you
what this option was about.
Inspired-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
It's an easy thing to do, but the for-loop ends up being pretty ugly, so
hide it behind the macro.
It would be even prettier with one of the (few) useful C99 features:
local for-loop variables. However, gcc needs special command line
options, and other compilers may not do it at all. So instead of doing
#define for_each_dive(_x) \
for (int _i = 0; ((_x) = get_dive(_i)) != NULL; _i++)
we require that the user declare the index iterator too, and the use
syntax becomes
for_each_dive(idx, dive) {
... use idx/dive here ...
}
And hey, maybe somebody actually will want to use the index, so maybe
that's not all bad.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The multi-dive case does fine, but the single-dive case (used when
adding a dive, for example) was somewhat confused between the dive index
(which is the location in the dive array) and the dive number.
Fix this by just passing the dive pointer instead (where NULL means to
use the current dive selection).
Reported-by: Jacco van Koll <jacco.van.koll@gmail.com>
Root-caused-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Now that the last commit tried to avoid changing the child selections if
the selected group partially matched, we should always [un]select all
children when we actually decide to change something.
Before, it would try to minimize selection damage by stopping
[un]selecting when it hit a child that already matched the selection,
but since we minimize damage differently, the all-or-nothing approach is
better, and gets us sane behavior when the group is collapsed.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This tries to avoid the problem mentioned in commit972669d6363c ("Rework
dive selection logic"), where a selection of dives hidden by collapsing
the group gets forgotten about by gtk. It does so by always marking the
group selected when it is collapsed with any selected children.
We also avoid selecting new children when a group is selected that
already has at least *some* children selected already. This way we do
minimal damage to existing selections when working with dive group
selections.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This completely changes how we keep track of selected dives: instead of
having an array listing the selection ("selectiontracker") or trusting
the gtk selection information, just save the information about whether a
dive is selected in the dive itself.
That makes it trivial to keep track of the state of selection across
group collapse/expand events, or when changing the tree view model. It
also ends up simplifying the code and logic in other ways.
HOWEVER, it does currently (re-)introduce an annoying oddity with gtk:
if you collapse a dive trip that has individual selections, gtk will
forget those selections ("out of sight, out of mind"), and when you do
*new* selections, the old hidden ones remain.
So there's some games required to make gtk do sane things. We may need
to either explicitly drop selections when collapsing trips, or make sure
the group entry gets selected when collapsing a group that has
selections in it. Or something.
There may be other issues introduced by this too.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This adds the ability to expand/collapse all the dive groupings in the
divelist from the divelist right-click context menu.
Should we perhaps add it to the top 'Dive' menu too?
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This shows the number of dives in the grup in the divelist header field,
and also picks the location from the first dive that *had* a location,
so that if any dive in the group has a valid location, the group will
have a location.
It also makes double-clicking a dive group expand/collapse that group.
Requested-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Pull dive selection fixes from Dirk Hohndel.
This hopefully fixes the common cases. Dirk is cursing gtk. We may
need some gtk selection guru to explain things.
* 'fixes' of git://git.hohndel.org/subsurface:
Another selection fix
More fiddling with the selection
The corner cases are getting more and more artificial. Without this patch,
the following can happen:
Select one or more dives in an (expanded) dive trip. Now collapse that
trip with the little triangle. Select a different trip. The previously
selected dive(s) are still part of the selection (as you can see, for
example, in the statistics tab).
With this patch the scenario above works as intended (all the dives in the
new trip are selected), but we have another corner case:
Just as before, select one or more dives in an expanded dive trip.
Collapse that trip and ctrl-click on another trip. Now you lose the
originally selected dives.
Frankly, if you ctrl-click to add more dives to your selection - just
don't collapse the trips the dives are in?
As this new corner case seems even more artificial than the previous one,
I consider this patch an improvement. But fundamentally I am just battling
all the ways in which gtk's selection handling is messed up. When I get
the selection call back I cannot tell if this is a new selection or an
incremental selection (i.e., a shift-click or ctrl-click).
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
As expected, this is pretty subtle to get right. But with this change the
code becomes simpler and more straight forward, I think. If the dives in a
group are collapsed, we don't even try to make gtk keep track of their
selection status - we explicitly do so ourselves. This avoids the
artificial expand / collapse around our attempt to force gtk to allow us
to select children that are hidden. But if a dive is expanded, then we
trust gtk to get things right.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
