[PATCH] Allow overlapping (and disjoint) dive trips
Linus Torvalds
torvalds at linux-foundation.org
Sun Dec 30 11:38:54 PST 2012
From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Sun, 30 Dec 2012 11:00:37 -0800
Subject: [PATCH] Allow overlapping (and disjoint) dive trips
We used to have the rule that a dive trip has to have all dives in it in
sequential order, even though our XML file really is much more flexible,
and allows arbitrary nesting of dives within a dive trip.
Put another way, the old model had fairly inflexible rules:
- the dive array is sorted by time
- a dive trip is always a contiguous slice of this sorted array
which makes perfect sense when you think of the dive and trip list as a
physical activity by one person, but leads to various very subtle issues
in the general case when there are no guarantees that the user then uses
subsurface that way.
In particular, if you load the XML files of two divers that have
overlapping dive trips, the end result is incredibly messy, and does not
conform to the above model at all.
There's two ways to enforce such conformance:
- disallow that kind of behavior entirely.
This is actually hard. Our XML files aren't date-based, they are
based on XML nesting rules, and even a single XML file can have
nesting that violates the date ordering. With multiple XML files,
it's trivial to do in practice, and while we could just fail at
loading, the failure would have to be a hard failure that leaves the
user no way to use the data at all.
- try to "fix it up" by sorting, splitting, and combining dive trips
automatically.
Dirk had a patch to do this, but it really does destroy the actual
dive data: if you load both mine and Dirk's dive trips, you ended up
with a result that followed the above two technical rules, but that
didn't actually make any *sense*.
So this patch doesn't try to enforce the rules, and instead just changes
them to be more generic:
- the dive array is still sorted by dive time
- a dive trip is just an arbitrary collection of dives.
The relaxed rules means that mixing dives and dive trips for two people
is trivial, and we can easily handle any XML file. The dive trip is
defined by the XML nesting level, and is totally independent of any
date-based sorting.
It does require a few things:
- when we save our dive data, we have to do it hierarchically by dive
trip, not just by walking the dive array linearly.
- similarly, when we create the dive tree model, we can't just blindly
walk the array of dives one by one, we have to look up the correct
trip (parent)
- when we try to merge two dives that are adjacent (by date sorting),
we can't do it if they are in different trips.
but apart from that, nothing else really changes.
NOTE! Despite the new relaxed model, creating totally disjoing dive
trips is not all that easy (nor is there any *reason* for it to be
easty). Our GUI interfaces still are "add dive to trip above" etc, and
the automatic adding of dives to dive trips is obviously still based on
date.
So this does not really change the expected normal usage, the relaxed
data structure rules just mean that we don't need to worry about the odd
cases as much, because we can just let them be.
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---
Relaxing this rule may well mean that we'd want to do other things too,
and also just allow removing an arbitrary dive from a trip (without
splitting the trip etc). This is a fairly minimal patch, and I haven't
gone through all the implications of this.
It's not heavily tested, but the testing I did was pretty relevant. Take
my and Dirk's dives (which, as dive buddies, have a lot of overlapping
dive trips), look at it, and save the end result (and re-load it).
Magically it all now actually makes sense (although it would make even
*more* sense if we added support for multiple people, and the trips would
show which person it is - right now the best way to do that is to look at
the dive computer ID ;)
So this adds a few more lines than it deletes, but it may well allow for
more future simpliciation of the interfaces, and it is a *lot* more
flexible.
dive.c | 4 ++++
dive.h | 1 +
divelist.c | 54 +++++++++++++++++++++++++++++++-----------------
save-xml.c | 70 +++++++++++++++++++++++++++++++++++++++++---------------------
4 files changed, 86 insertions(+), 43 deletions(-)
diff --git a/dive.c b/dive.c
index 1c7e140d4fe8..d49596532ece 100644
--- a/dive.c
+++ b/dive.c
@@ -1207,6 +1207,10 @@ static int likely_same_dive(struct dive *a, struct dive *b)
{
int fuzz, match;
+ /* Don't try to merge dives in different trips */
+ if (a->divetrip && b->divetrip && a->divetrip != b->divetrip)
+ return 0;
+
/*
* Do some basic sanity testing of the values we
* have filled in during 'fixup_dive()'
diff --git a/dive.h b/dive.h
index 1191ec6b6df4..355ec9ed774c 100644
--- a/dive.h
+++ b/dive.h
@@ -286,6 +286,7 @@ typedef struct dive_trip {
char *notes;
struct dive *dives;
int nrdives;
+ int index;
unsigned expanded:1, selected:1, autogen:1;
struct dive_trip *next;
} dive_trip_t;
diff --git a/divelist.c b/divelist.c
index 2e2599abc0bb..316c4d547a07 100644
--- a/divelist.c
+++ b/divelist.c
@@ -1199,12 +1199,19 @@ static void autogroup_dives(void)
#endif
}
+static void clear_trip_indexes(void)
+{
+ dive_trip_t *trip;
+
+ for (trip = dive_trip_list; trip != NULL; trip = trip->next)
+ trip->index = 0;
+}
+
static void fill_dive_list(void)
{
- int i;
- GtkTreeIter iter, parent_iter, *parent_ptr = NULL;
+ int i, trip_index = 0;
+ GtkTreeIter iter, parent_iter, lookup, *parent_ptr = NULL;
GtkTreeStore *liststore, *treestore;
- dive_trip_t *last_trip = NULL;
/* Do we need to create any dive groups automatically? */
if (autogroup)
@@ -1213,27 +1220,36 @@ static void fill_dive_list(void)
treestore = TREESTORE(dive_list);
liststore = LISTSTORE(dive_list);
+ clear_trip_indexes();
+
i = dive_table.nr;
while (--i >= 0) {
struct dive *dive = get_dive(i);
dive_trip_t *trip = dive->divetrip;
- if (trip != last_trip) {
- last_trip = trip;
- if (trip) {
- /* create trip entry */
- gtk_tree_store_append(treestore, &parent_iter, NULL);
- parent_ptr = &parent_iter;
- /* a duration of 0 (and negative index) identifies a group */
- gtk_tree_store_set(treestore, parent_ptr,
- DIVE_INDEX, -1,
- DIVE_DATE, trip->when,
- DIVE_LOCATION, trip->location,
- DIVE_DURATION, 0,
- -1);
- } else {
- parent_ptr = NULL;
- }
+ if (!trip) {
+ parent_ptr = NULL;
+ } else if (!trip->index) {
+ trip->index = ++trip_index;
+
+ /* Create new trip entry */
+ gtk_tree_store_append(treestore, &parent_iter, NULL);
+ parent_ptr = &parent_iter;
+
+ /* a duration of 0 (and negative index) identifies a group */
+ gtk_tree_store_set(treestore, parent_ptr,
+ DIVE_INDEX, -trip_index,
+ DIVE_DATE, trip->when,
+ DIVE_LOCATION, trip->location,
+ DIVE_DURATION, 0,
+ -1);
+ } else {
+ int index = trip->index;
+ GtkTreeModel *model = TREEMODEL(dive_list);
+ gtk_tree_model_get_iter_first(model, &lookup);
+ while (--index)
+ gtk_tree_model_iter_next(model, &lookup);
+ parent_ptr = &lookup;
}
/* store dive */
diff --git a/save-xml.c b/save-xml.c
index 45caefe4c638..eb0858c517bb 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -372,17 +372,6 @@ static void show_date(FILE *f, timestamp_t when)
tm.tm_hour, tm.tm_min, tm.tm_sec);
}
-static void save_trip(FILE *f, dive_trip_t *trip)
-{
- fprintf(f, "<trip");
- show_date(f, trip->when);
- if (trip->location)
- show_utf8(f, trip->location, " location=\'","\'", 1);
- fprintf(f, ">\n");
- if (trip->notes)
- show_utf8(f, trip->notes, "<notes>","</notes>\n", 0);
-}
-
static void save_samples(FILE *f, int nr, struct sample *s)
{
static const struct sample empty_sample;
@@ -443,6 +432,33 @@ static void save_dive(FILE *f, struct dive *dive)
fprintf(f, "</dive>\n");
}
+static void save_trip(FILE *f, dive_trip_t *trip)
+{
+ int i;
+ struct dive *dive;
+
+ fprintf(f, "<trip");
+ show_date(f, trip->when);
+ if (trip->location)
+ show_utf8(f, trip->location, " location=\'","\'", 1);
+ fprintf(f, ">\n");
+ if (trip->notes)
+ show_utf8(f, trip->notes, "<notes>","</notes>\n", 0);
+
+ /*
+ * Incredibly cheesy: we want to save the dives sorted, and they
+ * are sorted in the dive array.. So instead of using the dive
+ * list in the trip, we just traverse the global dive array and
+ * check the divetrip pointer..
+ */
+ for_each_dive(i, dive) {
+ if (dive->divetrip == trip)
+ save_dive(f, dive);
+ }
+
+ fprintf(f, "</trip>\n");
+}
+
static char *add_dc_to_string(char *dc_xml, struct divecomputer *dc)
{
char *pattern, *tmp;
@@ -481,7 +497,7 @@ void save_dives(const char *filename)
{
int i;
struct dive *dive;
- dive_trip_t *trip = NULL;
+ dive_trip_t *trip;
char *dc_xml = strdup("");
FILE *f = g_fopen(filename, "w");
@@ -502,22 +518,28 @@ void save_dives(const char *filename)
}
fprintf(f, dc_xml);
fprintf(f, "</settings>\n<dives>\n");
+
+ for (trip = dive_trip_list; trip != NULL; trip = trip->next)
+ trip->index = 0;
+
/* save the dives */
for_each_dive(i, dive) {
- dive_trip_t *thistrip = dive->divetrip;
- if (trip != thistrip) {
- /* Close the old trip? */
- if (trip)
- fprintf(f, "</trip>\n");
- /* Open the new one */
- if (thistrip)
- save_trip(f, thistrip);
- trip = thistrip;
+ trip = dive->divetrip;
+
+ /* Bare dive without a trip? */
+ if (!trip) {
+ save_dive(f, dive);
+ continue;
}
- save_dive(f, get_dive(i));
+
+ /* Have we already seen this trip (and thus saved this dive?) */
+ if (trip->index)
+ continue;
+
+ /* We haven't seen this trip before - save it and all dives */
+ trip->index = 1;
+ save_trip(f, trip);
}
- if (trip)
- fprintf(f, "</trip>\n");
fprintf(f, "</dives>\n</divelog>\n");
fclose(f);
}
--
1.8.1.rc2.6.g18499ba
More information about the subsurface
mailing list