Add SelectionController::GetSortedSelection and use it where useful

src/ass_karaoke.cpp

@@ -14,11 +14,6 @@
 //
 // Aegisub Project http://www.aegisub.org/
 
-/// @file ass_karaoke.cpp
-/// @brief Parse and manipulate ASSA karaoke tags
-/// @ingroup subs_storage
-///
-
 #include "ass_karaoke.h"
 
 #include "ass_dialogue.h"
@@ -266,51 +261,3 @@ void AssKaraoke::SetLineTimes(int start_time, int end_time) {
 	}
 	syls[idx].duration = end_time - syls[idx].start_time;
 }
-
-void AssKaraoke::SplitLines(std::set<AssDialogue*> const& lines, agi::Context *c) {
-	if (lines.empty()) return;
-
-	AssKaraoke kara;
-
-	Selection sel = c->selectionController->GetSelectedSet();
-
-	bool did_split = false;
-	for (auto it = c->ass->Events.begin(); it != c->ass->Events.end(); ++it) {
-		if (!lines.count(&*it)) continue;
-
-		kara.SetLine(&*it);
-
-		// If there aren't at least two tags there's nothing to split
-		if (kara.size() < 2) continue;
-
-		bool in_sel = sel.count(&*it) > 0;
-
-		for (auto const& syl : kara) {
-			auto new_line = new AssDialogue(*it);
-
-			new_line->Start = syl.start_time;
-			new_line->End = syl.start_time + syl.duration;
-			new_line->Text = syl.GetText(false);
-
-			c->ass->Events.insert(it, *new_line);
-
-			if (in_sel)
-				sel.insert(new_line);
-		}
-
-		--it; // Move `it` to the last of the new lines
-		sel.erase(&*it);
-		delete &*it;
-
-		did_split = true;
-	}
-
-	if (!did_split) return;
-
-	c->ass->Commit(_("splitting"), AssFile::COMMIT_DIAG_ADDREM | AssFile::COMMIT_DIAG_FULL);
-
-	AssDialogue *new_active = c->selectionController->GetActiveLine();
-	if (!sel.count(c->selectionController->GetActiveLine()))
-		new_active = *sel.begin();
-	c->selectionController->SetSelectionAndActive(std::move(sel), new_active);
-}

src/ass_karaoke.h

@@ -14,12 +14,6 @@
 //
 // Aegisub Project http://www.aegisub.org/
 
-/// @file ass_karaoke.h
-/// @see ass_karaoke.cpp
-/// @ingroup subs_storage
-///
-
-
 #include <map>
 #include <set>
 #include <string>
@@ -89,10 +83,5 @@ public:
 	/// Set the tag type for all karaoke tags in this line
 	void SetTagType(std::string const& new_type);
 
-	/// Split lines so that each syllable is its own line
-	/// @param lines Lines to split
-	/// @param c Project context
-	static void SplitLines(std::set<AssDialogue*> const& lines, agi::Context *c);
-
 	DEFINE_SIGNAL_ADDERS(AnnounceSyllablesChanged, AddSyllablesChangedListener)
 };

src/command/edit.cpp

@@ -542,12 +542,9 @@ struct edit_find_replace final : public Command {
 	}
 };
 
-static std::string get_entry_data(AssDialogue &d) { return d.GetEntryData(); }
 static void copy_lines(agi::Context *c) {
-	auto const& sel = c->selectionController->GetSelectedSet();
+	SetClipboard(join(c->selectionController->GetSortedSelection()
-	SetClipboard(join(c->ass->Events
+		| transformed(static_cast<std::string(*)(AssDialogue*)>([](AssDialogue *d) { return d->GetEntryData(); })),
-		| filtered([&](AssDialogue &d) { return sel.count(&d); })
-		| transformed(get_entry_data),
 		"\r\n"));
 }
 
@@ -749,20 +746,13 @@ struct edit_line_duplicate_shift_back final : public validate_video_and_sel_none
 };
 
 static void combine_lines(agi::Context *c, void (*combiner)(AssDialogue *, AssDialogue *), wxString const& message) {
-	auto const& sel = c->selectionController->GetSelectedSet();
+	auto sel = c->selectionController->GetSortedSelection();
 
-	AssDialogue *first = nullptr;
+	AssDialogue *first = sel[0];
-	for (auto it = c->ass->Events.begin(); it != c->ass->Events.end(); ) {
+	for (size_t i = 1; i < sel.size(); ++i) {
-		AssDialogue *diag = &*it++;
+		combiner(first, sel[1]);
-		if (!sel.count(diag)) continue;
+		first->End = std::max(first->End, sel[1]->End);
-		if (!first) {
+		delete sel[1];
-			first = diag;
-			continue;
-		}
-
-		combiner(first, diag);
-		first->End = std::max(first->End, diag->End);
-		delete diag;
 	}
 
 	c->selectionController->SetSelectionAndActive({first}, first);
@@ -912,15 +902,7 @@ struct edit_line_paste_over final : public Command {
 		}
 		else {
 			// Multiple lines selected, so paste over the selection
-
+			auto sorted_selection = c->selectionController->GetSortedSelection();
-			// Sort the selection by grid order
-			std::vector<AssDialogue*> sorted_selection;
-			sorted_selection.reserve(sel.size());
-			for (auto& line : c->ass->Events) {
-				if (sel.count(&line))
-					sorted_selection.push_back(&line);
-			}
-
 			auto pos = begin(sorted_selection);
 			paste_lines(c, true, [&](AssDialogue *new_line) -> AssDialogue * {
 				std::unique_ptr<AssDialogue> deleter(new_line);
@@ -1054,7 +1036,43 @@ struct edit_line_split_by_karaoke final : public validate_sel_nonempty {
 	STR_HELP("Use karaoke timing to split line into multiple smaller lines")
 
 	void operator()(agi::Context *c) override {
-		AssKaraoke::SplitLines(c->selectionController->GetSelectedSet(), c);
+		auto sel = c->selectionController->GetSortedSelection();
+		if (sel.empty()) return;
+
+		Selection new_sel;
+		AssKaraoke kara;
+
+		bool did_split = false;
+		for (auto line : sel) {
+			kara.SetLine(line);
+
+			// If there aren't at least two tags there's nothing to split
+			if (kara.size() < 2) continue;
+
+			for (auto const& syl : kara) {
+				auto new_line = new AssDialogue(*line);
+
+				new_line->Start = syl.start_time;
+				new_line->End = syl.start_time + syl.duration;
+				new_line->Text = syl.GetText(false);
+
+				c->ass->Events.insert(c->ass->iterator_to(*line), *new_line);
+
+				new_sel.insert(new_line);
+			}
+
+			delete line;
+			did_split = true;
+		}
+
+		if (!did_split) return;
+
+		c->ass->Commit(_("splitting"), AssFile::COMMIT_DIAG_ADDREM | AssFile::COMMIT_DIAG_FULL);
+
+		AssDialogue *new_active = c->selectionController->GetActiveLine();
+		if (!new_sel.count(c->selectionController->GetActiveLine()))
+			new_active = *sel.begin();
+		c->selectionController->SetSelectionAndActive(std::move(new_sel), new_active);
 	}
 };
 

src/command/time.cpp

@@ -60,16 +60,11 @@ namespace {
 	struct validate_adjoinable : public Command {
 		CMD_TYPE(COMMAND_VALIDATE)
 		bool Validate(const agi::Context *c) override {
-			auto const& sel = c->selectionController->GetSelectedSet();
+			auto sel = c->selectionController->GetSortedSelection();
-			if (sel.size() < 2) return !sel.empty();
+			if (sel.empty()) return false;
 
-			size_t found = 0;
+			for (size_t i = 1; i < sel.size(); ++i) {
-			for (auto& diag : c->ass->Events) {
+				if (sel[i]->Row != sel[i - 1]->Row + 1)
-				if (sel.count(&diag)) {
-					if (++found == sel.size())
-						return true;
-				}
-				else if (found)
 					return false;
 			}
 			return true;

src/dialog_selection.cpp

@@ -187,7 +187,7 @@ void DialogSelection::Process(wxCommandEvent&) {
 
 	Selection old_sel, new_sel;
 	if (action != Action::SET)
-		con->selectionController->GetSelectedSet(old_sel);
+		old_sel = con->selectionController->GetSelectedSet();
 
 	wxString message;
 	size_t count;

src/dialog_shift_times.cpp

@@ -346,18 +346,15 @@ void DialogShiftTimes::Process(wxCommandEvent &) {
 		shift = -shift;
 
 	// Track which rows were shifted for the log
-	int row_number = 0;
 	int block_start = 0;
 	json::Array shifted_blocks;
 
 	for (auto& line : context->ass->Events) {
-		++row_number;
-
 		if (!sel.count(&line)) {
 			if (block_start) {
 				json::Object block;
 				block["start"] = block_start;
-				block["end"] = row_number - 1;
+				block["end"] = line.Row;
 				shifted_blocks.push_back(block);
 				block_start = 0;
 			}
@@ -365,7 +362,7 @@ void DialogShiftTimes::Process(wxCommandEvent &) {
 			if (mode == 2 && shifted_blocks.empty()) continue;
 		}
 		else if (!block_start)
-			block_start = row_number;
+			block_start = line.Row + 1;
 
 		if (start)
 			line.Start = Shift(line.Start, shift, by_time, agi::vfr::START);
@@ -378,7 +375,7 @@ void DialogShiftTimes::Process(wxCommandEvent &) {
 	if (block_start) {
 		json::Object block;
 		block["start"] = block_start;
-		block["end"] = row_number - 1;
+		block["end"] = context->ass->Events.back().Row + 1;
 		shifted_blocks.push_back(block);
 	}
 

src/dialog_timing_processor.cpp

@@ -309,9 +309,8 @@ std::vector<AssDialogue*> DialogTimingProcessor::SortDialogues() {
 	// Check if rows are valid
 	for (auto diag : sorted) {
 		if (diag->Start > diag->End) {
-			int line = std::distance(c->ass->Events.begin(), c->ass->iterator_to(*diag));
 			wxMessageBox(
-				wxString::Format(_("One of the lines in the file (%i) has negative duration. Aborting."), line),
+				wxString::Format(_("One of the lines in the file (%i) has negative duration. Aborting."), diag->Row),
 				_("Invalid script"),
 				wxOK | wxICON_ERROR | wxCENTER);
 			sorted.clear();

src/selection_controller.cpp

@@ -22,6 +22,8 @@
 #include "subs_controller.h"
 #include "utils.h"
 
+#include <algorithm>
+
 SelectionController::SelectionController(agi::Context *c)
 : context(c)
 , open_connection(c->subsController->AddFileOpenListener(&SelectionController::OnSubtitlesOpen, this))
@@ -69,6 +71,12 @@ void SelectionController::SetSelectionAndActive(Selection new_selection, AssDial
 		AnnounceActiveLineChanged(new_line);
 }
 
+std::vector<AssDialogue *> SelectionController::GetSortedSelection() const {
+	std::vector<AssDialogue *> ret(selection.begin(), selection.end());
+	sort(begin(ret), end(ret), [](AssDialogue *a, AssDialogue *b) { return a->Row < b->Row; });
+	return ret;
+}
+
 void SelectionController::PrevLine() {
 	if (!active_line) return;
 	auto it = context->ass->iterator_to(*active_line);

src/selection_controller.h

@@ -30,6 +30,7 @@
 #include <libaegisub/signal.h>
 
 #include <set>
+#include <vector>
 
 class AssDialogue;
 typedef std::set<AssDialogue *> Selection;
@@ -82,14 +83,13 @@ public:
 	/// be sent.
 	void SetSelectedSet(Selection new_selection);
 
-	/// @brief Obtain the selected set
-	/// @param[out] selection Filled with the selected set on return
-	void GetSelectedSet(Selection &out) const { out = selection; }
-
 	/// @brief Obtain the selected set
 	/// @return The selected set
 	Selection const& GetSelectedSet() const { return selection; }
 
+	/// Get the selection sorted by row number
+	std::vector<AssDialogue *> GetSortedSelection() const;
+
 	/// @brief Set both the selected set and active line
 	/// @param new_line Subtitle line to become the new active line
 	/// @param new_selection The set of subtitle lines to become the new selected set