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@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

4 files changed, 86 insertions, 43 deletions

diff --git a/dive.c b/dive.c
index 081c7eb25..98b96b3ff 100644
--- a/dive.c
+++ b/dive.c
@@ -1209,6 +1209,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 34b1ec549..91694f8c3 100644
--- a/dive.h
+++ b/dive.h
@@ -287,6 +287,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 2e2599abc..316c4d547 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 61469e9ce..0cfc7ccfd 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -374,17 +374,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;
@@ -445,6 +434,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;
@@ -483,7 +499,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");
@@ -504,22 +520,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);
 }