pt_import: since we aren't using undo here, need to delete the diff_command manually

get_edit_op_range() already checks for the existence of a Range

gtk2_ardour/edit_note_dialog.cc

@@ -245,12 +245,12 @@ EditNoteDialog::done (int r)
 
 	}
 
-	if (!had_change) {
+	if (had_change) {
-		_region_view->abort_command ();
+		_region_view->apply_note_diff ();
+	} else {
+		_region_view->abort_note_diff ();
 	}
 
-	_region_view->apply_diff ();
-
 	list<Evoral::event_id_t> notes;
 	for (set<NoteBase*>::iterator i = _events.begin(); i != _events.end(); ++i) {
 		notes.push_back ((*i)->note()->id());

gtk2_ardour/editor.h

@@ -1362,7 +1362,6 @@ private:
 	void reverse_region ();
 	void strip_region_silence ();
 	void normalize_region ();
-	void reset_region_scale_amplitude ();
 	void adjust_region_gain (bool up);
 	void reset_region_gain ();
 	void quantize_region ();

gtk2_ardour/editor_actions.cc

@@ -1882,7 +1882,6 @@ Editor::register_region_actions ()
 	register_region_action (_region_actions, RegionActionTarget (SelectedRegions), "spectral-analyze-region", _("Spectral Analysis..."), sigc::mem_fun (*this, &Editor::spectral_analyze_region_selection));
 
 	register_region_action (_region_actions, RegionActionTarget (SelectedRegions|EnteredRegions), "reset-region-gain-envelopes", _("Reset Envelope"), sigc::mem_fun (*this, &Editor::reset_region_gain_envelopes));
-	register_region_action (_region_actions, RegionActionTarget (SelectedRegions|EnteredRegions), "reset-region-scale-amplitude", _("Reset Gain"), sigc::mem_fun (*this, &Editor::reset_region_scale_amplitude));
 
 	register_toggle_region_action (_region_actions, RegionActionTarget (SelectedRegions|EnteredRegions), "toggle-region-gain-envelope-active", _("Envelope Active"), sigc::mem_fun (*this, &Editor::toggle_gain_envelope_active));
 

gtk2_ardour/editor_drag.cc

@@ -2831,7 +2831,7 @@ NoteResizeDrag::finished (GdkEvent* event, bool movement_occurred)
 				sd = _snap_delta;
 			}
 
-			mrv->commit_resizing (nb, at_front, _drags->current_pointer_x() - grab_x(), relative, sd, snap);
+			mrv->finish_resizing (nb, at_front, _drags->current_pointer_x() - grab_x(), relative, sd, snap);
 		}
 	}
 
@@ -6909,11 +6909,8 @@ NoteCreateDrag::finished (GdkEvent* ev, bool had_movement)
 		length = length.round_to_subdivision (div, RoundUpMaybe);
 	}
 
-#warning NUTEMPO ALERT not snapping correctly
+	/* create_note_at() implements UNDO for us */
-
-	_editor->begin_reversible_command (_("Create Note"));
 	_region_view->create_note_at (timepos_t (start), _drag_rect->y0(), length, ev->button.state, false);
-	_editor->commit_reversible_command ();
 }
 
 double
@@ -6962,8 +6959,9 @@ HitCreateDrag::start_grab (GdkEvent* event, Gdk::Cursor* cursor)
 	const Temporal::Beats start = beats - _region_view->region()->position().beats ();
 	Temporal::Beats length = _region_view->get_draw_length_beats (pos);
 
-	_editor->begin_reversible_command (_("Create Hit"));
 	_region_view->clear_note_selection();
+
+	/* create_note_at() implements UNDO for us */
 	_region_view->create_note_at (timepos_t (start), _y, length, event->button.state, false);
 
 	_last_pos = timepos_t (start);

gtk2_ardour/editor_ops.cc

@@ -2321,7 +2321,7 @@ Editor::set_session_end_from_playhead ()
 
 		XMLNode &after = loc->get_state();
 
-		begin_reversible_command (_("Set session start"));
+		begin_reversible_command (_("Set session end"));
 
 		_session->add_command (new MementoCommand<Location>(*loc, &before, &after));
 
@@ -2509,15 +2509,17 @@ Editor::set_mark ()
 void
 Editor::clear_markers ()
 {
-	if (_session) {
+	if (!_session) {
-		begin_reversible_command (_("clear markers"));
+		return;
+	}
 
-		XMLNode &before = _session->locations()->get_state();
+	begin_reversible_command (_("clear markers"));
-		if (_session->locations()->clear_markers ()) {
+
-			XMLNode &after = _session->locations()->get_state();
+	XMLNode &before = _session->locations()->get_state();
-			_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
+	if (_session->locations()->clear_markers ()) {
-			commit_reversible_command ();
+		XMLNode &after = _session->locations()->get_state();
-		}
+		_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
+		commit_reversible_command ();
 	} else {
 		abort_reversible_command ();
 	}
@@ -2526,16 +2528,18 @@ Editor::clear_markers ()
 void
 Editor::clear_xrun_markers ()
 {
-	if (_session) {
+	if (!_session) {
-		begin_reversible_command (_("clear xrun markers"));
+		return;
+	}
 
-		XMLNode &before = _session->locations()->get_state();
+	begin_reversible_command (_("clear xrun markers"));
-		if (_session->locations()->clear_xrun_markers ()) {
-			XMLNode &after = _session->locations()->get_state();
-			_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
 
-			commit_reversible_command ();
+	XMLNode &before = _session->locations()->get_state();
-		}
+	if (_session->locations()->clear_xrun_markers ()) {
+		XMLNode &after = _session->locations()->get_state();
+		_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
+
+		commit_reversible_command ();
 	} else {
 		abort_reversible_command ();
 	}
@@ -2544,18 +2548,20 @@ Editor::clear_xrun_markers ()
 void
 Editor::clear_ranges ()
 {
-	if (_session) {
+	if (!_session) {
-		begin_reversible_command (_("clear ranges"));
+		return;
+	}
 
-		XMLNode &before = _session->locations()->get_state();
+	begin_reversible_command (_("clear ranges"));
 
-		if (_session->locations()->clear_ranges ()) {
+	XMLNode &before = _session->locations()->get_state();
 
-			XMLNode &after = _session->locations()->get_state();
+	if (_session->locations()->clear_ranges ()) {
-			_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
 
-			commit_reversible_command ();
+		XMLNode &after = _session->locations()->get_state();
-		}
+		_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
+
+		commit_reversible_command ();
 	} else {
 		abort_reversible_command ();
 	}
@@ -3446,27 +3452,16 @@ Editor::separate_under_selected_regions ()
 void
 Editor::crop_region_to_selection ()
 {
-	if (!selection->time.empty()) {
+	timepos_t start;
+	timepos_t end;
+
+	if (get_edit_op_range (start, end)) {
+		begin_reversible_command (_("Crop Regions to Edit Range"));
+
+		crop_region_to (start, end);
 
-		begin_reversible_command (_("Crop Regions to Time Selection"));
-		for (std::list<TimelineRange>::iterator i = selection->time.begin(); i != selection->time.end(); ++i) {
-			crop_region_to ((*i).start(), (*i).end());
-		}
 		commit_reversible_command();
-	} else {
-
-		timepos_t start;
-		timepos_t end;
-
-		if (get_edit_op_range (start, end)) {
-			begin_reversible_command (_("Crop Regions to Edit Range"));
-
-			crop_region_to (start, end);
-
-			commit_reversible_command();
-		}
 	}
-
 }
 
 void
@@ -3895,11 +3890,8 @@ Editor::trim_region_to_location (const Location& loc, const char* str)
 	for (RegionSelection::iterator x = rs.begin(); x != rs.end(); ++x) {
 		RegionView* rv = (*x);
 
-		/* require region to span proposed trim */
+		/* require selected region(s) to overlap with proposed trim */
-		switch (rv->region()->coverage (loc.start(), loc.end())) {
+		if (rv->region()->coverage (loc.start(), loc.end())==Temporal::OverlapNone) {
-		case Temporal::OverlapNone:
-			break;
-		default:
 			continue;
 		}
 
@@ -5173,6 +5165,7 @@ Editor::paste_internal (timepos_t const & pos, float times)
 		   R1.A1, R1.A2, R2, R2.A1, ... */
 	}
 
+	bool commit = false;
 	begin_reversible_command (Operations::paste);
 
 	if (ts.size() == 1 && cut_buffer->lines.size() == 1 &&
@@ -5181,7 +5174,7 @@ Editor::paste_internal (timepos_t const & pos, float times)
 	       "greedy" paste from one automation type to another. */
 
 		PasteContext ctx(paste_count, times, ItemCounts(), true);
-		ts.front()->paste (position, *cut_buffer, ctx);
+		commit |= ts.front()->paste (position, *cut_buffer, ctx);
 
 	} else {
 
@@ -5189,13 +5182,17 @@ Editor::paste_internal (timepos_t const & pos, float times)
 
 		PasteContext ctx(paste_count, times, ItemCounts(), false);
 		for (TrackViewList::iterator i = ts.begin(); i != ts.end(); ++i) {
-			(*i)->paste (position, *cut_buffer, ctx);
+			commit |= (*i)->paste (position, *cut_buffer, ctx);
 		}
 	}
 
 	++paste_count;
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5688,41 +5685,6 @@ Editor::normalize_region ()
 	}
 }
 
-
-void
-Editor::reset_region_scale_amplitude ()
-{
-	if (!_session) {
-		return;
-	}
-
-	RegionSelection rs = get_regions_from_selection_and_entered ();
-
-	if (rs.empty()) {
-		return;
-	}
-
-	bool in_command = false;
-
-	for (RegionSelection::iterator r = rs.begin(); r != rs.end(); ++r) {
-		AudioRegionView* const arv = dynamic_cast<AudioRegionView*>(*r);
-		if (!arv)
-			continue;
-		arv->region()->clear_changes ();
-		arv->audio_region()->set_scale_amplitude (1.0f);
-
-		if(!in_command) {
-				begin_reversible_command ("reset gain");
-				in_command = true;
-		}
-		_session->add_command (new StatefulDiffCommand (arv->region()));
-	}
-
-	if (in_command) {
-		commit_reversible_command ();
-	}
-}
-
 void
 Editor::adjust_region_gain (bool up)
 {
@@ -8703,7 +8665,7 @@ Editor::toggle_region_mute ()
 
 	for (RegionSelection::iterator i = rs.begin(); i != rs.end(); ++i) {
 
-		(*i)->region()->playlist()->clear_changes ();
+		(*i)->region()->clear_changes ();
 		(*i)->region()->set_muted (!(*i)->region()->muted ());
 		_session->add_command (new StatefulDiffCommand ((*i)->region()));
 

gtk2_ardour/editor_selection.cc

@@ -1576,7 +1576,7 @@ Editor::sensitize_the_right_region_actions (bool because_canvas_crossing)
 	}
 
 	if (!have_non_unity_scale_amplitude || !have_audio) {
-		_region_actions->get_action("reset-region-scale-amplitude")->set_sensitive (false);
+		_region_actions->get_action("reset-region-gain")->set_sensitive (false);
 	}
 
 	Glib::RefPtr<ToggleAction> a = Glib::RefPtr<ToggleAction>::cast_dynamic (_region_actions->get_action("toggle-region-lock"));

gtk2_ardour/midi_list_editor.cc

@@ -362,7 +362,7 @@ MidiListEditor::scroll_event (GdkEventScroll* ev)
 			}
 		}
 
-		m->apply_command (*_session, cmd);
+		m->apply_diff_command_as_commit (*_session, cmd);
 
 		/* reset selection to be as it was before we rebuilt */
 
@@ -465,7 +465,7 @@ MidiListEditor::key_release (GdkEventKey* ev)
 		note = (*iter)[columns._note];
 		copy.reset (new NoteType (*note.get()));
 		cmd->add (copy);
-		m->apply_command (*_session, cmd);
+		m->apply_diff_command_as_commit (*_session, cmd);
 		/* model has been redisplayed by now */
 		path.next ();
 		/* select, start editing column 2 (note) */
@@ -531,7 +531,7 @@ MidiListEditor::delete_selected_note ()
 		cmd->remove (*i);
 	}
 
-	m->apply_command (*_session, cmd);
+	m->apply_diff_command_as_commit (*_session, cmd);
 }
 
 void
@@ -729,7 +729,7 @@ MidiListEditor::edited (const std::string& path, const std::string& text)
 			}
 		}
 
-		m->apply_command (*_session, cmd);
+		m->apply_diff_command_as_commit (*_session, cmd);
 
 		/* model has been redisplayed by now */
 		/* keep selected row(s), move cursor there, don't continue editing */

gtk2_ardour/midi_region_view.cc

@@ -801,7 +801,7 @@ MidiRegionView::channel_edit ()
 		i = next;
 	}
 
-	apply_diff ();
+	apply_note_diff ();
 }
 
 void
@@ -837,7 +837,7 @@ MidiRegionView::velocity_edit ()
 		i = next;
 	}
 
-	apply_diff ();
+	apply_note_diff ();
 }
 
 void
@@ -888,10 +888,8 @@ MidiRegionView::create_note_at (timepos_t const & t, double y, Temporal::Beats l
 	view->update_note_range(new_note->note());
 
 	start_note_diff_command(_("add note"));
-
 	note_diff_add_note (new_note, true, false);
-
+	apply_note_diff();
-	apply_diff();
 
 	trackview.editor().set_selected_midi_region_view (*this);
 	list<Evoral::event_id_t> to_be_selected;
@@ -940,6 +938,8 @@ MidiRegionView::start_note_diff_command (string name)
 	if (!_note_diff_command) {
 		trackview.editor().begin_reversible_command (name);
 		_note_diff_command = _model->new_note_diff_command (name);
+	} else {
+		std::cerr << "ERROR: start_note_diff_command command called, but a note_diff_command was already underway" << std::endl;
 	}
 }
 
@@ -986,10 +986,8 @@ MidiRegionView::note_diff_add_change (NoteBase* ev,
 }
 
 void
-MidiRegionView::apply_diff (bool as_subcommand, bool was_copy)
+MidiRegionView::apply_note_diff (bool as_subcommand, bool was_copy)
 {
-	bool commit = false;
-
 	if (!_note_diff_command) {
 		return;
 	}
@@ -1003,11 +1001,11 @@ MidiRegionView::apply_diff (bool as_subcommand, bool was_copy)
 		}
 	}
 
-	if (as_subcommand) {
+	/*note that we don't use as_commit here, because that would BEGIN a new undo record; we already have one underway*/
-		_model->apply_command_as_subcommand (*trackview.session(), _note_diff_command);
+	_model->apply_diff_command_as_subcommand (*trackview.session(), _note_diff_command);
-	} else {
+
-		_model->apply_command (*trackview.session(), _note_diff_command);
+	if (!as_subcommand) {
-		commit = true;
+		trackview.editor().commit_reversible_command ();  /*instead, we can explicitly commit the command in progress */
 	}
 
 	_note_diff_command = 0;
@@ -1017,14 +1015,10 @@ MidiRegionView::apply_diff (bool as_subcommand, bool was_copy)
 	}
 
 	_marked_for_velocity.clear();
-
-	if (commit) {
-		trackview.editor().commit_reversible_command ();
-	}
 }
 
 void
-MidiRegionView::abort_command()
+MidiRegionView::abort_note_diff()
 {
 	delete _note_diff_command;
 	_note_diff_command = 0;
@@ -2005,7 +1999,7 @@ MidiRegionView::step_add_note (uint8_t channel, uint8_t number, uint8_t velocity
 	clear_selection_internal ();
 	note_diff_add_note (new_note, true, false);
 
-	apply_diff();
+	apply_note_diff();
 
 	// last_step_edit_note = new_note;
 }
@@ -2094,7 +2088,6 @@ void
 MidiRegionView::change_patch_change (PatchChange& pc, const MIDI::Name::PatchPrimaryKey& new_patch)
 {
 	string name = _("alter patch change");
-	trackview.editor().begin_reversible_command (name);
 
 	MidiModel::PatchChangeDiffCommand* c = _model->new_patch_change_diff_command (name);
 
@@ -2107,8 +2100,7 @@ MidiRegionView::change_patch_change (PatchChange& pc, const MIDI::Name::PatchPri
 		c->change_bank (pc.patch (), new_bank);
 	}
 
-	_model->apply_command (*trackview.session(), c);
+	_model->apply_diff_command_as_commit (*trackview.session(), c);
-	trackview.editor().commit_reversible_command ();
 
 	remove_canvas_patch_change (&pc);
 	display_patch_changes ();
@@ -2118,7 +2110,7 @@ void
 MidiRegionView::change_patch_change (MidiModel::PatchChangePtr old_change, const Evoral::PatchChange<Temporal::Beats> & new_change)
 {
 	string name = _("alter patch change");
-	trackview.editor().begin_reversible_command (name);
+
 	MidiModel::PatchChangeDiffCommand* c = _model->new_patch_change_diff_command (name);
 
 	if (old_change->time() != new_change.time()) {
@@ -2137,8 +2129,7 @@ MidiRegionView::change_patch_change (MidiModel::PatchChangePtr old_change, const
 		c->change_bank (old_change, new_change.bank());
 	}
 
-	_model->apply_command (*trackview.session(), c);
+	_model->apply_diff_command_as_commit (*trackview.session(), c);
-	trackview.editor().commit_reversible_command ();
 
 	for (PatchChanges::iterator x = _patch_changes.begin(); x != _patch_changes.end(); ++x) {
 		if (x->second->patch() == old_change) {
@@ -2160,7 +2151,6 @@ MidiRegionView::add_patch_change (timecnt_t const & t, Evoral::PatchChange<Tempo
 {
 	string name = _("add patch change");
 
-	trackview.editor().begin_reversible_command (name);
 	MidiModel::PatchChangeDiffCommand* c = _model->new_patch_change_diff_command (name);
 
 	c->add (MidiModel::PatchChangePtr (
@@ -2168,8 +2158,7 @@ MidiRegionView::add_patch_change (timecnt_t const & t, Evoral::PatchChange<Tempo
 		        (_region->source_relative_position (_region->position() + t).beats(),
 		         patch.channel(), patch.program(), patch.bank())));
 
-	_model->apply_command (*trackview.session(), c);
+	_model->apply_diff_command_as_commit (*trackview.session(), c);
-	trackview.editor().commit_reversible_command ();
 
 	display_patch_changes ();
 }
@@ -2177,11 +2166,9 @@ MidiRegionView::add_patch_change (timecnt_t const & t, Evoral::PatchChange<Tempo
 void
 MidiRegionView::move_patch_change (PatchChange& pc, Temporal::Beats t)
 {
-	trackview.editor().begin_reversible_command (_("move patch change"));
 	MidiModel::PatchChangeDiffCommand* c = _model->new_patch_change_diff_command (_("move patch change"));
 	c->change_time (pc.patch (), t);
-	_model->apply_command (*trackview.session(), c);
+	_model->apply_diff_command_as_commit (*trackview.session(), c);
-	trackview.editor().commit_reversible_command ();
 
 	display_patch_changes ();
 }
@@ -2189,12 +2176,9 @@ MidiRegionView::move_patch_change (PatchChange& pc, Temporal::Beats t)
 void
 MidiRegionView::delete_patch_change (PatchChange* pc)
 {
-	trackview.editor().begin_reversible_command (_("delete patch change"));
-
 	MidiModel::PatchChangeDiffCommand* c = _model->new_patch_change_diff_command (_("delete patch change"));
 	c->remove (pc->patch ());
-	_model->apply_command (*trackview.session(), c);
+	_model->apply_diff_command_as_commit (*trackview.session(), c);
-	trackview.editor().commit_reversible_command ();
 
 	remove_canvas_patch_change (pc);
 	display_patch_changes ();
@@ -2247,7 +2231,7 @@ MidiRegionView::delete_selection()
 
 	_selection.clear();
 
-	apply_diff ();
+	apply_note_diff ();
 
 	hide_verbose_cursor ();
 }
@@ -2257,7 +2241,7 @@ MidiRegionView::delete_note (boost::shared_ptr<NoteType> n)
 {
 	start_note_diff_command (_("delete note"));
 	_note_diff_command->remove (n);
-	apply_diff ();
+	apply_note_diff ();
 
 	hide_verbose_cursor ();
 }
@@ -2935,7 +2919,8 @@ MidiRegionView::note_dropped(NoteBase *, timecnt_t const & d_qn, int8_t dnote, b
 		_copy_drag_events.clear ();
 	}
 
-	apply_diff (false, copy);
+	apply_note_diff (true /*subcommand, we don't want this to start a new commit*/, copy);
+	trackview.editor().commit_reversible_command ();
 
 	// care about notes being moved beyond the upper/lower bounds on the canvas
 	if (lowest_note_in_selection  < midi_stream_view()->lowest_note() ||
@@ -3144,9 +3129,9 @@ MidiRegionView::update_resizing (NoteBase* primary, bool at_front, double delta_
  *  Parameters the same as for \a update_resizing().
  */
 void
-MidiRegionView::commit_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, bool with_snap)
+MidiRegionView::finish_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, bool with_snap)
 {
-	_note_diff_command = _model->new_note_diff_command (_("resize notes"));
+	_note_diff_command = _model->new_note_diff_command (_("resize notes"));  /* we are a subcommand, so we don't want to use start_note_diff() which begins a new command */
 
 	/* XX why doesn't snap_pixel_to_sample() handle this properly? */
 	bool const ensure_snap = trackview.editor().snap_mode () != SnapMagnetic;
@@ -3227,7 +3212,7 @@ MidiRegionView::commit_resizing (NoteBase* primary, bool at_front, double delta_
 	}
 
 	_resize_data.clear();
-	apply_diff(true);
+	apply_note_diff(true/*subcommand*/);
 }
 
 void
@@ -3445,7 +3430,7 @@ MidiRegionView::change_velocities (bool up, bool fine, bool allow_smush, bool al
 		i = next;
 	}
 
-	apply_diff();
+	apply_note_diff();
 
   cursor_label:
 	if (!_selection.empty()) {
@@ -3504,7 +3489,7 @@ MidiRegionView::transpose (bool up, bool fine, bool allow_smush)
 		i = next;
 	}
 
-	apply_diff ();
+	apply_note_diff ();
 
 	if (lowest < midi_stream_view()->lowest_note() || highest > midi_stream_view()->highest_note()) {
 		midi_stream_view()->update_note_range (lowest);
@@ -3543,7 +3528,7 @@ MidiRegionView::change_note_lengths (bool fine, bool shorter, Temporal::Beats de
 		i = next;
 	}
 
-	apply_diff ();
+	apply_note_diff ();
 
 }
 
@@ -3604,7 +3589,7 @@ MidiRegionView::nudge_notes (bool forward, bool fine)
 		i = next;
 	}
 
-	apply_diff ();
+	apply_note_diff ();
 }
 
 void
@@ -3615,7 +3600,7 @@ MidiRegionView::change_channel(uint8_t channel)
 		note_diff_add_change (*i, MidiModel::NoteDiffCommand::Channel, channel);
 	}
 
-	apply_diff();
+	apply_note_diff();
 }
 
 
@@ -3798,7 +3783,7 @@ MidiRegionView::cut_copy_clear (Editing::CutCopyOp op)
 			}
 		}
 
-		apply_diff();
+		apply_note_diff();
 	}
 }
 
@@ -3821,7 +3806,7 @@ MidiRegionView::selection_as_cut_buffer () const
 void
 MidiRegionView::duplicate_selection ()
 {
-	std::cerr << "dup selection\n";
+	trackview.editor().begin_reversible_command (_("duplicate notes"));
 
 	if (_selection.empty()) {
 		return;
@@ -3849,10 +3834,15 @@ MidiRegionView::duplicate_selection ()
 	local_selection.set (note_selection);
 
 	PasteContext ctxt (0, 1, ItemCounts(), false);
-	paste (dup_pos, local_selection, ctxt);
+	bool commit = paste (dup_pos, local_selection, ctxt);
+	if (commit) {
+		trackview.editor().commit_reversible_command ();
+	} else {
+		trackview.editor().abort_reversible_command ();
+	}
 }
 
-/** This method handles undo */
+/** undo commands were initiated at the 'action' level. ::paste and ::paste_internal should implement subcommands */
 bool
 MidiRegionView::paste (timepos_t const & pos, const ::Selection& selection, PasteContext& ctx)
 {
@@ -3880,20 +3870,14 @@ MidiRegionView::paste (timepos_t const & pos, const ::Selection& selection, Past
 	const ATracks& atracks = midi_view()->automation_tracks();
 	for (ATracks::const_iterator a = atracks.begin(); a != atracks.end(); ++a) {
 		if (a->second->paste(pos, selection, ctx)) {
-			if(!commit) {
-				trackview.editor().begin_reversible_command (Operations::paste);
-			}
 			commit = true;
 		}
 	}
 
-	if (commit) {
+	return commit;
-		trackview.editor().commit_reversible_command ();
-	}
-	return true;
 }
 
-/** This method handles undo */
+/** undo commands were initiated at the 'action' level. ::paste and ::paste_internal should implement subcommands */
 void
 MidiRegionView::paste_internal (timepos_t const & pos, unsigned paste_count, float times, const MidiCutBuffer& mcb)
 {
@@ -3903,7 +3887,7 @@ MidiRegionView::paste_internal (timepos_t const & pos, unsigned paste_count, flo
 
 	PBD::Unwinder<bool> puw (_pasting, true);
 
-	start_note_diff_command (_("paste"));
+	MidiModel::NoteDiffCommand* cmd = _model->new_note_diff_command (_("paste")); /* we are a subcommand, so we don't want to use start_note_diff */
 
 	const Temporal::Beats snap_beats    = get_grid_beats(pos);
 	const Temporal::Beats first_time    = (*mcb.notes().begin())->time();
@@ -3953,7 +3937,7 @@ MidiRegionView::paste_internal (timepos_t const & pos, unsigned paste_count, flo
 	_marked_for_selection.clear ();
 	_pending_note_selection.clear ();
 
-	apply_diff (true);
+	_model->apply_diff_command_as_subcommand(*trackview.session(), cmd);
 }
 
 struct EventNoteTimeEarlyFirstComparator {

gtk2_ardour/midi_region_view.h

@@ -182,14 +182,17 @@ public:
 
 	void display_model(boost::shared_ptr<ARDOUR::MidiModel> model);
 
+	/* note_diff commands should start here; this initiates an undo record */
 	void start_note_diff_command (std::string name = "midi edit");
+
 	void note_diff_add_change (NoteBase* ev, ARDOUR::MidiModel::NoteDiffCommand::Property, uint8_t val);
 	void note_diff_add_change (NoteBase* ev, ARDOUR::MidiModel::NoteDiffCommand::Property, Temporal::Beats val);
 	void note_diff_add_note (const boost::shared_ptr<NoteType> note, bool selected, bool show_velocity = false);
 	void note_diff_remove_note (NoteBase* ev);
 
-	void apply_diff (bool as_subcommand = false, bool was_copy = false);
+	/* note_diff commands should be completed with one of these calls; they may (or may not) commit the undo record */
-	void abort_command();
+	void apply_note_diff (bool as_subcommand = false, bool was_copy = false);
+	void abort_note_diff();
 
 	void   note_entered(NoteBase* ev);
 	void   note_left(NoteBase* ev);
@@ -241,7 +244,7 @@ public:
 	void begin_resizing(bool at_front);
 
 	void update_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, bool with_snap);
-	void commit_resizing (NoteBase* primary, bool at_front, double delat_x, bool relative, double snap_delta, bool with_snap);
+	void finish_resizing (NoteBase* primary, bool at_front, double delat_x, bool relative, double snap_delta, bool with_snap);
 	void abort_resizing ();
 
 	/** Change the channel of the selection.

libs/ardour/ardour/midi_model.h

@@ -256,7 +256,7 @@ public:
 	 *
 	 * This has no side-effects on the model or Session, the returned command
 	 * can be held on to for as long as the caller wishes, or discarded without
-	 * formality, until apply_command is called and ownership is taken.
+	 * formality, until apply_diff_command_* is called and ownership is taken.
 	 */
 	MidiModel::NoteDiffCommand* new_note_diff_command (const std::string& name = "midi edit");
 	/** Start a new SysExDiff command */
@@ -268,18 +268,25 @@ public:
 	/** Apply a command.
 	 *
 	 * Ownership of cmd is taken, it must not be deleted by the caller.
+	 * This STARTS and COMMITS an undo command.
 	 * The command will constitute one item on the undo stack.
 	 */
-	void apply_command (Session& session, Command* cmd);
+	void apply_diff_command_as_commit (Session& session, Command* cmd);
 
-	void apply_command (Session* session, Command* cmd) { if (session) { apply_command (*session, cmd); } }
+	void apply_diff_command_as_commit (Session* session, Command* cmd) { if (session) { apply_diff_command_as_commit (*session, cmd); } }
 
-	/** Apply a command as part of a larger reversible transaction
+	/** Add a command as part of a larger reversible transaction
 	 *
 	 * Ownership of cmd is taken, it must not be deleted by the caller.
-	 * The command will constitute one item on the undo stack.
+	 * The command will be incorporated into the current command.
 	 */
-	void apply_command_as_subcommand (Session& session, Command* cmd);
+	void apply_diff_command_as_subcommand (Session& session, Command* cmd);
+
+	/** Apply the midi diff, but without any effect on undo
+	 *
+	 * Ownership of cmd is not changed.
+	 */
+	void apply_diff_command_only (Session& session, Command* cmd);
 
 	bool sync_to_source (const Source::WriterLock& source_lock);
 

libs/ardour/import_pt.cc

@@ -461,7 +461,8 @@ no_audio_tracks:
 			/* PT C-2 = 0, Ardour C-1 = 0, subtract twelve to convert ? */
 			midicmd->add (boost::shared_ptr<Evoral::Note<Temporal::Beats> > (new Evoral::Note<Temporal::Beats> ((uint8_t)1, start, len, j->note, j->velocity)));
 		}
-		mm->apply_command (this, midicmd);
+		mm->apply_diff_command_only (*this, midicmd);
+		delete midicmd;
 		boost::shared_ptr<Region> copy (RegionFactory::create (mr, true));
 		playlist->clear_changes ();
 		playlist->add_region (copy, timepos_t (f));

libs/ardour/luabindings.cc

@@ -1571,7 +1571,8 @@ LuaBindings::common (lua_State* L)
 		.endClass ()
 
 		.deriveWSPtrClass <MidiModel, AutomatableSequence<Temporal::Beats> > ("MidiModel")
-		.addFunction ("apply_command", (void (MidiModel::*)(Session*, Command*))&MidiModel::apply_command)
+		.addFunction ("apply_command", (void (MidiModel::*)(Session*, Command*))&MidiModel::apply_diff_command_as_commit) /* deprecated: left here in case any extant scripts use apply_command */
+		.addFunction ("apply_diff_command_as_commit", (void (MidiModel::*)(Session*, Command*))&MidiModel::apply_diff_command_as_commit)
 		.addFunction ("new_note_diff_command", &MidiModel::new_note_diff_command)
 		.endClass ()
 

libs/ardour/midi_model.cc

@@ -92,7 +92,7 @@ MidiModel::new_patch_change_diff_command (const string& name)
 
 
 void
-MidiModel::apply_command(Session& session, Command* cmd)
+MidiModel::apply_diff_command_as_commit(Session& session, Command* cmd)
 {
 	session.begin_reversible_command (cmd->name());
 	(*cmd)();
@@ -101,13 +101,20 @@ MidiModel::apply_command(Session& session, Command* cmd)
 }
 
 void
-MidiModel::apply_command_as_subcommand(Session& session, Command* cmd)
+MidiModel::apply_diff_command_as_subcommand(Session& session, Command* cmd)
 {
 	(*cmd)();
 	session.add_command (cmd);
 	set_edited (true);
 }
 
+void
+MidiModel::apply_diff_command_only(Session& session, Command* cmd)
+{
+	(*cmd)();
+	set_edited (true);
+}
+
 /* ************* DIFF COMMAND ********************/
 
 #define NOTE_DIFF_COMMAND_ELEMENT "NoteDiffCommand"
@@ -1702,7 +1709,7 @@ MidiModel::insert_silence_at_start (TimeType t)
 			c->change (*i, NoteDiffCommand::StartTime, (*i)->time() + t);
 		}
 
-		apply_command_as_subcommand (_midi_source.session(), c);
+		apply_diff_command_as_subcommand (_midi_source.session(), c);
 	}
 
 	/* Patch changes */
@@ -1714,7 +1721,7 @@ MidiModel::insert_silence_at_start (TimeType t)
 			c->change_time (*i, (*i)->time() + t);
 		}
 
-		apply_command_as_subcommand (_midi_source.session(), c);
+		apply_diff_command_as_subcommand (_midi_source.session(), c);
 	}
 
 	/* Controllers */
@@ -1736,7 +1743,7 @@ MidiModel::insert_silence_at_start (TimeType t)
 			c->change (*i, (*i)->time() + t);
 		}
 
-		apply_command_as_subcommand (_midi_source.session(), c);
+		apply_diff_command_as_subcommand (_midi_source.session(), c);
 	}
 
 	ContentsShifted (timecnt_t (t));

libs/ardour/session_state.cc

@@ -3161,6 +3161,14 @@ Session::begin_reversible_command (const string& name)
 void
 Session::begin_reversible_command (GQuark q)
 {
+	if (_current_trans) {
+		cerr << "An UNDO transaction was started while a prior command was underway. Aborting command (" << g_quark_to_string (q) << ") and prior (" << _current_trans->name() << ")" << endl;
+		PBD::warning << "An UNDO transaction was started while a prior command was underway. Aborting command (" << g_quark_to_string (q) << ") and prior (" << _current_trans->name() << ")" << endmsg;
+		abort_reversible_command();
+		assert (false);
+		return;
+	}
+
 	/* If nested begin/commit pairs are used, we create just one UndoTransaction
 	   to hold all the commands that are committed.  This keeps the order of
 	   commands correct in the history.