Undo: implement undo of tag editing

The code follows the other edit-commands, but uses its own base
class, because it is distinctly different. Editing the tag field
does not simply mean setting the tag for all dives, but rather
adding and removing individual tags.

This class will be reused for editing of dive buddies and masters.

Modify the tag widget thus that it sends an editingFinished()
signal when it goes out of focus. The editingFinished() signal
was prevented by hooking into the return, enter and tab key-events.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

desktop-widgets/command.cpp

@@ -175,4 +175,9 @@ void editDiveSiteNew(const QVector<dive *> dives, const QString &newName, struct
 	execute(new EditDiveSiteNew(dives, newName, oldValue));
 }
 
+void editTags(const QVector<dive *> &dives, const QStringList &newList, struct dive *d)
+{
+	execute(new EditTags(dives, newList, d));
+}
+
 } // namespace Command

desktop-widgets/command.h

@@ -61,6 +61,7 @@ void editAirTemp(const QVector<dive *> dives, int newValue, int oldValue);
 void editWaterTemp(const QVector<dive *> dives, int newValue, int oldValue);
 void editDiveSite(const QVector<dive *> dives, struct dive_site *newValue, struct dive_site *oldValue);
 void editDiveSiteNew(const QVector<dive *> dives, const QString &newName, struct dive_site *oldValue);
+void editTags(const QVector<dive *> &dives, const QStringList &newList, struct dive *d);
 
 } // namespace Command
 

desktop-widgets/command_edit.cpp

@@ -325,4 +325,129 @@ DiveField EditMode::fieldId() const
 	return DiveField::MODE;
 }
 
+// ***** Tag based commands *****
+EditTagsBase::EditTagsBase(const QVector<dive *> &divesIn, const QStringList &newListIn, struct dive *d):
+	dives(divesIn.toStdVector()),
+	newList(newListIn),
+	oldDive(d)
+{
+}
+
+// Two helper functions: returns true if first list contains any tag or
+// misses any tag of second list.
+static bool containsAny(const QStringList &tags1, const QStringList &tags2)
+{
+	return std::any_of(tags2.begin(), tags2.end(), [&tags1](const QString &tag)
+			   { return tags1.contains(tag); });
+}
+
+static bool missesAny(const QStringList &tags1, const QStringList &tags2)
+{
+	return std::any_of(tags2.begin(), tags2.end(), [&tags1](const QString &tag)
+			   { return !tags1.contains(tag); });
+}
+
+// This is quite hackish: we can't use virtual functions in the constructor and
+// therefore can't initialize the list of dives [the values of the dives are
+// accessed by virtual functions]. Therefore, we (mis)use the fact that workToBeDone()
+// is called exactly once before adding the Command to the system and perform this here.
+// To be more explicit about this, we might think about renaming workToBeDone() to init().
+bool EditTagsBase::workToBeDone()
+{
+	// changing the tags on multiple dives is semantically strange - what's the right thing to do?
+	// here's what I think... add the tags that were added to the displayed dive and remove the tags
+	// that were removed from it
+
+	// Calculate tags to add and tags to remove
+	QStringList oldList = data(oldDive);
+	for (const QString &s: newList) {
+		if (!oldList.contains(s))
+			tagsToAdd.push_back(s);
+	}
+	for (const QString &s: oldList) {
+		if (!newList.contains(s))
+			tagsToRemove.push_back(s);
+	}
+
+	// Now search for all dives that either
+	//	- miss a tag to be added
+	//	- have a tag to be removed
+	std::vector<dive *> divesNew;
+	divesNew.reserve(dives.size());
+	for (dive *d: dives) {
+		QStringList tags = data(d);
+		if (missesAny(tags, tagsToAdd) || containsAny(tags, tagsToRemove))
+			divesNew.push_back(d);
+	}
+	dives = std::move(divesNew);
+
+	// Create a text for the menu entry. In the case of multiple dives add the number
+	size_t num_dives = dives.size();
+	if (num_dives > 0)
+		//: remove the part in parantheses for %n = 1
+		setText(tr("Edit %1 (%n dive(s))", "", num_dives).arg(fieldName()));
+
+	return num_dives;
+}
+
+void EditTagsBase::undo()
+{
+	if (dives.empty()) {
+		qWarning("Edit command called with empty dives list (shouldn't happen)");
+		return;
+	}
+
+	for (dive *d: dives) {
+		QStringList tags = data(d);
+		for (const QString &tag: tagsToRemove)
+			tags.removeAll(tag);
+		for (const QString &tag: tagsToAdd) {
+			if (!tags.contains(tag))
+				tags.push_back(tag);
+		}
+		invalidate_dive_cache(d); // Ensure that dive is written in git_save()
+		set(d, tags);
+	}
+
+	std::swap(tagsToAdd, tagsToRemove);
+
+	emit diveListNotifier.divesEdited(QVector<dive *>::fromStdVector(dives), fieldId());
+
+	mark_divelist_changed(true);
+}
+
+// Undo and redo do the same as just the stored value is exchanged
+void EditTagsBase::redo()
+{
+	undo();
+}
+
+// ***** Tags *****
+QStringList EditTags::data(struct dive *d) const
+{
+	QStringList res;
+	for (const struct tag_entry *tag = d->tag_list; tag; tag = tag->next)
+		res.push_back(tag->tag->name);
+	return res;
+}
+
+void EditTags::set(struct dive *d, const QStringList &v) const
+{
+	taglist_free(d->tag_list);
+	d->tag_list = NULL;
+	for (const QString &tag: v)
+		taglist_add_tag(&d->tag_list, qPrintable(tag));
+	taglist_cleanup(&d->tag_list);
+}
+
+QString EditTags::fieldName() const
+{
+	return tr("tags");
+}
+
+DiveField EditTags::fieldId() const
+{
+	return DiveField::TAGS;
+}
+
 } // namespace Command

desktop-widgets/command_edit.h

@@ -136,6 +136,43 @@ public:
 	DiveField fieldId() const override;
 };
 
+// Fields that work with tag-lists (tags, buddies, divemasters) work differently and therefore
+// have their own base class. In this case, it's not a template, as all these lists are base
+// on strings.
+class EditTagsBase : public Base {
+	bool workToBeDone() override;
+
+	// Dives to be edited. For historical reasons, the *last* entry was
+	// the active dive when the user initialized the action. This dive
+	// will be made the current dive on redo / undo.
+	std::vector<dive *> dives;
+	QStringList newList;	// Temporary until initialized
+	struct dive *oldDive;	// Temporary until initialized
+public:
+	EditTagsBase(const QVector<dive *> &dives, const QStringList &newList, struct dive *d);
+
+protected:
+	QStringList tagsToAdd;
+	QStringList tagsToRemove;
+	void undo() override;
+	void redo() override;
+
+	// Getters, setters and parsers to be overriden by sub-classes.
+	virtual QStringList data(struct dive *d) const = 0;
+	virtual void set(struct dive *d, const QStringList &v) const = 0;
+	virtual QString fieldName() const = 0;	// Name of the field, used to create the undo menu-entry
+	virtual DiveField fieldId() const = 0;
+};
+
+class EditTags : public EditTagsBase {
+public:
+	using EditTagsBase::EditTagsBase;	// Use constructor of base class.
+	QStringList data(struct dive *d) const override;
+	void set(struct dive *d, const QStringList &v) const override;
+	QString fieldName() const override;
+	DiveField fieldId() const override;
+};
+
 } // namespace Command
 
 #endif

desktop-widgets/tab-widgets/maintab.cpp

@@ -372,6 +372,9 @@ void MainTab::divesEdited(const QVector<dive *> &, DiveField field)
 		updateDiveSite(current_dive);
 		emit diveSiteChanged();
 		break;
+	case DiveField::TAGS:
+		ui.tagWidget->setText(get_taglist_string(current_dive->tag_list));
+		break;
 	default:
 		break;
 	}
@@ -788,7 +791,6 @@ void MainTab::acceptChanges()
 		// three text fields are somewhat special and are represented as tags
 		// in the UI - they need somewhat smarter handling
 		saveTaggedStrings(selectedDives);
-		saveTags(selectedDives);
 
 		if (cylindersModel->changed) {
 			mark_divelist_changed(true);
@@ -1108,57 +1110,11 @@ void MainTab::copyTagsToDisplayedDive()
 {
 	taglist_free(displayed_dive.tag_list);
 	displayed_dive.tag_list = NULL;
-	Q_FOREACH (const QString& tag, ui.tagWidget->getBlockStringList())
+	Q_FOREACH (const QString &tag, ui.tagWidget->getBlockStringList())
 		taglist_add_tag(&displayed_dive.tag_list, qPrintable(tag));
 	taglist_cleanup(&displayed_dive.tag_list);
 }
 
-// changing the tags on multiple dives is semantically strange - what's the right thing to do?
-// here's what I think... add the tags that were added to the displayed dive and remove the tags
-// that were removed from it
-void MainTab::saveTags(const QVector<dive *> &selectedDives)
-{
-	struct dive *cd = current_dive;
-	struct tag_entry *added_list = NULL;
-	struct tag_entry *removed_list = NULL;
-	struct tag_entry *tl;
-
-	copyTagsToDisplayedDive();
-
-	// figure out which tags were added and which tags were removed
-	added_list = taglist_added(cd ? cd->tag_list : NULL, displayed_dive.tag_list);
-	removed_list = taglist_added(displayed_dive.tag_list, cd ? cd->tag_list : NULL);
-
-	// dump_taglist("added tags:", added_list);
-	// dump_taglist("removed tags:", removed_list);
-
-	// we need to check if the tags were changed before just overwriting them
-	if (added_list == NULL && removed_list == NULL)
-		return;
-
-	MODIFY_DIVES(selectedDives,
-		// create a new tag list and all the existing tags that were not
-		// removed and then all the added tags
-		struct tag_entry *new_tag_list;
-		new_tag_list = NULL;
-		tl = mydive->tag_list;
-		while (tl) {
-			if (!taglist_contains(removed_list, tl->tag->name))
-				taglist_add_tag(&new_tag_list, tl->tag->name);
-			tl = tl->next;
-		}
-		tl = added_list;
-		while (tl) {
-			taglist_add_tag(&new_tag_list, tl->tag->name);
-			tl = tl->next;
-		}
-		taglist_free(mydive->tag_list);
-		mydive->tag_list = new_tag_list;
-	);
-	taglist_free(added_list);
-	taglist_free(removed_list);
-}
-
 // buddy and divemaster are represented in the UI just like the tags, but the internal
 // representation is just a string (with commas as delimiters). So we need to do the same
 // thing we did for tags, just differently
@@ -1226,15 +1182,12 @@ int MainTab::diffTaggedStrings(QString currentString, QString displayedString, Q
 	return removedList.length() + addedList.length();
 }
 
-void MainTab::on_tagWidget_textChanged()
+void MainTab::on_tagWidget_editingFinished()
 {
-	if (editMode == IGNORE || acceptingEdit == true)
+	if (editMode == IGNORE || acceptingEdit == true || !current_dive)
 		return;
 
-	if (get_taglist_string(displayed_dive.tag_list) == ui.tagWidget->toPlainText())
-		return;
-
-	markChangedWidget(ui.tagWidget);
+	Command::editTags(getSelectedDivesCurrentLast(), ui.tagWidget->getBlockStringList(), current_dive);
 }
 
 void MainTab::on_location_diveSiteSelected()

desktop-widgets/tab-widgets/maintab.h

@@ -89,7 +89,7 @@ slots:
 	void on_timeEdit_timeChanged(const QTime & time);
 	void on_rating_valueChanged(int value);
 	void on_visibility_valueChanged(int value);
-	void on_tagWidget_textChanged();
+	void on_tagWidget_editingFinished();
 	void editCylinderWidget(const QModelIndex &index);
 	void editWeightWidget(const QModelIndex &index);
 	void addDiveStarted();
@@ -119,7 +119,6 @@ private:
 	int lastTabSelectedDiveTrip;
 	void resetPallete();
 	void copyTagsToDisplayedDive();
-	void saveTags(const QVector<dive *> &selectedDives);
 	void saveTaggedStrings(const QVector<dive *> &selectedDives);
 	int diffTaggedStrings(QString currentString, QString displayedString, QStringList &addedList, QStringList &removedList);
 	void markChangedWidget(QWidget *w);

desktop-widgets/tagwidget.cpp

@@ -211,3 +211,11 @@ void TagWidget::fixPopupPosition(int delta)
 		m_completer->popup()->setGeometry(toGlobal.x(), toGlobal.y() + delta +10, toGlobal.width(), toGlobal.height());
 	}
 }
+
+// Since we capture enter / return / tab, we never send an editingFinished() signal.
+// Therefore, override the focusOutEvent()
+void TagWidget::focusOutEvent(QFocusEvent *ev)
+{
+	GroupedLineEdit::focusOutEvent(ev);
+	emit editingFinished();
+}

desktop-widgets/tagwidget.h

@@ -28,6 +28,7 @@ slots:
 protected:
 	void keyPressEvent(QKeyEvent *e);
 private:
+	void focusOutEvent(QFocusEvent *ev) override;
 	QCompleter *m_completer;
 	bool lastFinishedTag;
 };