From c833ca1b78609a88d94da84100f7b359a8936640 Mon Sep 17 00:00:00 2001
From: Petr Gladkikh <petrglad@gmail.com>
Date: Thu, 30 Nov 2023 01:10:48 +0100
Subject: [PATCH] Diff command history revamp

Remaining issues:
* Tape cut does not work as expected.
* The code needs cleanup, some parts are unused now.
* The history starts with empty track (undoing the first change clears
the track - that may surprize for users).
---
 README.md            |  11 +-
 src/app.rs           |   2 +-
 src/changeset.rs     |  38 ++--
 src/edit_commands.rs | 436 +++++++++++++++++++++++++++++++++++++++++
 src/main.rs          |   1 +
 src/stave.rs         | 212 ++++++++------------
 src/track.rs         |  68 ++++---
 src/track_edit.rs    | 447 +++++++++++++++++--------------------------
 src/track_history.rs | 150 ++++++++-------
 src/util.rs          |  17 +-
 10 files changed, 846 insertions(+), 536 deletions(-)
 create mode 100644 src/edit_commands.rs

diff --git a/README.md b/README.md
index 8a5a6ba..663bb31 100644
--- a/README.md
+++ b/README.md
@@ -2,7 +2,7 @@
 
 Off grid MIDI editor with following goals:
 
-* Not a DAW (would like it to be but do not have enough time for that).
+* Not a DAW: MIDI is the input, MIDI is exported (would like it to be but do not have enough time for that).
 * Do not care (much) about measures. Primarily aimed at piano real recordings without strict tempo/bars.
 * A feature absent in other midi editors I could get my hands on (both commercial and free ones): removing a piece
   of MIDI recording as one can remove a time fragment from PCM recording. For some reason DAW authors insist on handling
@@ -12,9 +12,9 @@ Off grid MIDI editor with following goals:
 * Comfortable workflow with keyboard.
 * Allows making fine adjustments of notes and tempo.
 * Unlimited undo/redo. Never loose session data. Non destructive edits, do not override original files.
-* Blackbox recording (always-on MIDI recording).
+* Flight recorder (always-on MIDI recording).
 
-I'd love to see this in one of commercial or open-source DAWs and even pay money for that, but I do not see it
+I'd love this to be in one of commercial or open-source DAWs and even pay money for that, but I do not see it
 happening.
 
 ## Status
@@ -35,14 +35,17 @@ I use Pianoteq, but that is a commercial product.
 
 ## TODO
 
+- [ ] Organize commands, reduce diff disk usage.
+- [ ] Highlight undo changes.
+- [ ] Location history navigation (e.g. go to a bookmark that was visited recently), with Alt + LeftArrow / RightArrow
 - [ ] Adjust tempo for selection.
 - [ ] When start playing send current CC values (will help damper to take effect immediately, not on next change). 
 - [ ] Time marks on stave ("minute:second" from the beginning).
-- [ ] Location history navigation (e.g. go to a bookmark that was visited recently), with Alt + LeftArrow / RightArrow
 - [ ] Minimize use of unwrap. The biggest contention currently is event data shared between engine and stave.
 - [ ] Multi-track UI (for snippets, flight recorder, and copy/paste buffer). Can show only one at a time, though. Use
   tabs?
 - [ ] Show (scroll to) changing objects before undo/redo.
+- [ ] Reduce number of range types (preferring util::Range?) 
 - [ ] Zoom to fit whole composition.
 - [ ] Visual hint for out-of-view selected notes. Scroll to the earliest of the selected notes on an action, if none of
   them are currently visible.
diff --git a/src/app.rs b/src/app.rs
index 0790313..f510bff 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -99,7 +99,7 @@ impl EmApp {
 
 impl eframe::App for EmApp {
     fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
-        self.stave.history.do_pending();
+        // TODO (tokio, diff history) self.stave.history.do_pending();
         if let Some(message) = self.message_receiver.try_iter().last() {
             match message {
                 Message::UpdateTime(t) => {
diff --git a/src/changeset.rs b/src/changeset.rs
index e03b0ac..023c4d7 100644
--- a/src/changeset.rs
+++ b/src/changeset.rs
@@ -3,15 +3,18 @@ use std::collections::HashMap;
 use serde::{Deserialize, Serialize};
 
 use crate::common::VersionId;
+use crate::edit_commands::{CommandDiff, EditCommandId};
 use crate::track::{EventId, Note, Track, TrackEvent, TrackEventType};
 
-/// Simplest track edit operation. See Changeset for uses.
+/// Simplest track edit operation. See [Changeset] for uses.
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 pub enum EventAction {
-    // TODO It is possible to recover necessary state by patching one of the recent (preferably the
-    //   most recent) snapshots. Such snapshots (the ones that track event ids) are not
-    //   implemented yet, so adding "before" states here to support undo operations
-    //   as the initial draft in-memory implementation.
+    /* Adding "before" states here to support undo operations (the EventAction itself has
+    enough information to undo).
+    TODO (possible revamp) Alternatively it is also possible to recover necessary state by patching
+     one of the recent snapshots. That approach may probably help to save space and simplify
+     data structures. E.g. Delete action will only need the event id and update action will
+     only need the new state. OTOH then we'll need to save snapshots more often. */
     Delete(TrackEvent),
     Update(TrackEvent, TrackEvent),
     Insert(TrackEvent),
@@ -41,16 +44,28 @@ impl EventAction {
             EventAction::Insert(_) => None,
         }
     }
+
+    pub fn revert(&self) -> Self {
+        match self {
+            EventAction::Delete(ev) => EventAction::Insert(ev.clone()),
+            EventAction::Update(before, after) => {
+                EventAction::Update(after.clone(), before.clone())
+            }
+            EventAction::Insert(ev) => EventAction::Delete(ev.clone()),
+        }
+    }
 }
 
 /// Complete patch of a track editing action.
-/// TODO This should be a part of the persisted edit history, then it should contain the complete event values instead of ids.
-///   Note that this would also require event ids that are unique within the whole project history (the generator value should be)
+/// Plain [EventActionsList] can be used instead, but there the actions order becomes important
+/// (e.g. duplicating 'update' actions will overwrite previous result).
 #[derive(Debug)]
 pub struct Changeset {
     pub changes: HashMap<EventId, EventAction>,
 }
 
+pub type EventActionsList = Vec<EventAction>;
+
 impl Changeset {
     pub fn empty() -> Self {
         Changeset {
@@ -62,7 +77,7 @@ impl Changeset {
         self.changes.insert(action.event_id(), action);
     }
 
-    pub fn add_all(&mut self, actions: &Vec<EventAction>) {
+    pub fn add_all(&mut self, actions: &EventActionsList) {
         for a in actions.iter().cloned() {
             self.add(a);
         }
@@ -77,14 +92,15 @@ impl Changeset {
 /// undo hints (so it is obvious what is currently changing), and avoid storing whole track
 /// every time. See also [Snapshot], [Changeset].
 #[derive(Serialize, Deserialize)]
-pub struct Patch {
+pub struct HistoryLogEntry {
     pub base_version: VersionId,
     pub version: VersionId,
-    pub changes: Vec<EventAction>,
+    pub command_id: EditCommandId,
+    pub diff: Vec<CommandDiff>,
 }
 
 /// Serializable snapshot of a complete track state that can be exported or used as a base
-/// for Patch sequence. See also [Patch].
+/// for Patch sequence. See also [HistoryLogEntry].
 #[derive(Serialize, Deserialize)]
 pub struct Snapshot {
     pub version: VersionId,
diff --git a/src/edit_commands.rs b/src/edit_commands.rs
new file mode 100644
index 0000000..4647e16
--- /dev/null
+++ b/src/edit_commands.rs
@@ -0,0 +1,436 @@
+use std::collections::HashSet;
+
+use serde::{Deserialize, Serialize};
+
+use crate::changeset::{Changeset, EventAction, EventActionsList};
+use crate::common::Time;
+use crate::stave::PIANO_KEY_LINES;
+use crate::track::{
+    is_cc_switch_on, ControllerId, ControllerSetValue, EventId, Level, Note, Pitch, Track,
+    TrackEvent, TrackEventType, MAX_LEVEL, MIDI_CC_SUSTAIN_ID,
+};
+use crate::util::{range_contains, IdSeq, Range};
+
+#[derive(Serialize, Deserialize)]
+pub enum EditCommandId {
+    ShiftTail,
+    TapeInsert,
+    TapeDelete,
+    AddNote,
+    DeleteEvents,
+    SetDamper,
+    SetDamperOn,
+    EventsShift,
+    NotesStretch,
+    NotesTranspose,
+    NotesAccent,
+    Load,
+}
+
+// Commands that do not usually generate large patches can use generic Changeset, this is the default.
+// Commands that cannot be stored efficiently should use custom diffs.
+// Note to support undo, the event updates must be reversible.
+#[derive(Debug, Serialize, Deserialize)]
+pub enum CommandDiff {
+    Changeset { patch: EventActionsList },
+    TailShift { at: Time, delta: Time },
+}
+
+// TODO (cleanup) Use a constant, ctx field, or a setting for delta value (see KEYBOARD_TIME_STEP).
+const TIME_DELTA: Time = 10_000;
+
+pub type AppliedCommand = (EditCommandId, Vec<CommandDiff>);
+
+pub fn into_changeset(track: &Track, diffs: &Vec<CommandDiff>, changeset: &mut Changeset) {
+    for d in diffs {
+        apply_diff(track, d, changeset);
+    }
+}
+
+pub fn apply_diff(track: &Track, diff: &CommandDiff, changeset: &mut Changeset) {
+    match diff {
+        CommandDiff::Changeset { patch } => changeset.add_all(patch),
+        CommandDiff::TailShift { at, delta } => do_shift_tail(track, at, &delta, changeset),
+    }
+}
+
+pub fn revert_diffs(track: &Track, diffs: &Vec<CommandDiff>, changeset: &mut Changeset) {
+    for d in diffs.iter().rev() {
+        revert_diff(track, d, changeset);
+    }
+}
+
+pub fn revert_diff(track: &Track, diff: &CommandDiff, changeset: &mut Changeset) {
+    match diff {
+        CommandDiff::Changeset { patch } => {
+            for action in patch.iter().rev() {
+                changeset.add(action.revert())
+            }
+        }
+        CommandDiff::TailShift { at, delta } => do_shift_tail(track, at, &-delta, changeset),
+    }
+}
+
+pub fn shift_tail(at: &Time, delta: &Time) -> (EditCommandId, Vec<CommandDiff>) {
+    (
+        EditCommandId::ShiftTail,
+        vec![CommandDiff::TailShift {
+            at: *at,
+            delta: *delta,
+        }],
+    )
+}
+
+// TODO (cleanup) Organize the naming? Functions updating changeset, vs functions producing CommandDiffs
+fn do_shift_tail(track: &Track, at: &Time, delta: &Time, changeset: &mut Changeset) {
+    // TODO (improvement) When moving earlier adjust last delta so the events will start exactly at 'at'.
+    let mut actions = vec![];
+    for ev in &track.events {
+        if *at < ev.at {
+            if (ev.at + delta) < *at {
+                /* The action should be replayable and undoable. If we allow events to move earlier
+                then 'at' time, then on undo we should somehow find them still while not confusing them
+                with unchanged events in (at - delta, at] range (when if delta > 0).
+                Track events are expected to be in sorted order, so this will likely trigger early. */
+                break;
+            }
+            actions.push(shift_event(&ev, delta));
+        }
+    }
+    changeset.add_all(&actions);
+}
+
+fn do_delete_events_range(track: &Track, range: &Range<Time>, changeset: &mut Changeset) {
+    for ev in &track.events {
+        if range_contains(range, ev.at) {
+            changeset.add(EventAction::Delete(ev.clone()));
+        }
+    }
+}
+
+pub fn tape_insert(range: &Range<Time>) -> AppliedCommand {
+    let mut diffs = vec![];
+    let delta = range.1 - range.0;
+    assert!(delta >= 0);
+    diffs.push(CommandDiff::TailShift { at: range.0, delta });
+    (EditCommandId::TapeInsert, diffs)
+}
+
+pub fn tape_delete(track: &Track, range: &Range<Time>) -> AppliedCommand {
+    let mut patch = vec![];
+    for ev in &track.events {
+        if range_contains(range, ev.at) {
+            patch.push(EventAction::Delete(ev.clone()));
+        }
+    }
+    let delta = range.1 - range.0;
+    assert!(delta >= 0);
+    (
+        EditCommandId::TapeInsert,
+        vec![
+            CommandDiff::Changeset { patch },
+            CommandDiff::TailShift {
+                at: range.0,
+                delta: -delta,
+            },
+        ],
+    )
+}
+
+fn edit_selected(
+    track: &Track,
+    selection: &HashSet<EventId>,
+    action: &dyn Fn(&TrackEvent) -> Option<EventAction>,
+) -> Vec<CommandDiff> {
+    let mut patch = vec![];
+    for ev in &track.events {
+        if selection.contains(&ev.id) {
+            if let Some(action) = action(&ev) {
+                patch.push(action);
+            }
+        }
+    }
+    vec![CommandDiff::Changeset { patch }]
+}
+
+fn edit_selected_notes<'a, Action: Fn(&Note) -> Option<Note>>(
+    track: &Track,
+    selection: &HashSet<EventId>,
+    action: &'a Action,
+) -> Vec<CommandDiff> {
+    // Adapt note action to be an event action.
+    let event_action = move |ev: &TrackEvent| {
+        if let TrackEvent {
+            event: TrackEventType::Note(n),
+            ..
+        } = &ev
+        {
+            if let Some(n) = action(n) {
+                let mut ev = ev.clone();
+                ev.event = TrackEventType::Note(n);
+                return Some(EventAction::Update(ev.clone(), ev));
+            }
+        }
+        None
+    };
+    edit_selected(track, selection, &event_action)
+}
+
+pub fn delete_selected(track: &Track, selection: &HashSet<EventId>) -> AppliedCommand {
+    let diff = edit_selected(track, selection, &|ev| {
+        Some(EventAction::Delete(ev.clone()))
+    });
+    (EditCommandId::DeleteEvents, diff)
+}
+
+fn shift_event(ev: &TrackEvent, delta: &Time) -> EventAction {
+    let mut nev = ev.clone();
+    nev.at += delta;
+    EventAction::Update(ev.clone(), nev)
+}
+
+pub fn shift_selected(track: &Track, selection: &HashSet<EventId>, delta: &Time) -> AppliedCommand {
+    let diff = edit_selected(track, selection, &|ev| Some(shift_event(ev, delta)));
+    (EditCommandId::EventsShift, diff)
+}
+
+pub fn stretch_selected_notes(
+    track: &Track,
+    selection: &HashSet<EventId>,
+    delta: &Time,
+) -> AppliedCommand {
+    let diff = edit_selected_notes(track, selection, &|note: &Note| {
+        let mut note = note.clone();
+        note.duration += delta;
+        Some(note)
+    });
+    (EditCommandId::NotesStretch, diff)
+}
+
+pub fn transpose_selected_notes(
+    track: &Track,
+    selection: &HashSet<EventId>,
+    delta: i8,
+) -> AppliedCommand {
+    let diff = edit_selected_notes(track, selection, &|note: &Note| {
+        let mut note = note.clone();
+        if let Some(x) = note.pitch.checked_add_signed(delta) {
+            if PIANO_KEY_LINES.contains(&x) {
+                note.pitch = x;
+                return Some(note);
+            }
+        }
+        None
+    });
+    (EditCommandId::NotesTranspose, diff)
+}
+
+pub fn accent_selected_notes(
+    track: &Track,
+    selection: &HashSet<EventId>,
+    delta: i8,
+) -> AppliedCommand {
+    let diff = edit_selected_notes(track, selection, &|note: &Note| {
+        if let Some(pitch) = note.velocity.checked_add_signed(delta) {
+            let mut note = note.clone();
+            note.velocity = pitch;
+            Some(note)
+        } else {
+            None
+        }
+    });
+    (EditCommandId::NotesAccent, diff)
+}
+
+pub fn add_new_note(id_seq: &IdSeq, range: &Range<Time>, pitch: &Pitch) -> AppliedCommand {
+    let mut diff = vec![];
+    assert!(range.1 - range.0 > 0);
+    diff.push(CommandDiff::Changeset {
+        patch: vec![EventAction::Insert(TrackEvent {
+            id: id_seq.next(),
+            at: range.0,
+            event: TrackEventType::Note(Note {
+                pitch: *pitch,
+                velocity: MAX_LEVEL / 2,
+                duration: range.1 - range.0,
+            }),
+        })],
+    });
+    (EditCommandId::AddNote, diff)
+}
+
+fn sustain_event(id_seq: &IdSeq, at: &Time, on: bool) -> TrackEvent {
+    TrackEvent {
+        id: id_seq.next(),
+        at: *at,
+        event: TrackEventType::Controller(ControllerSetValue {
+            controller_id: MIDI_CC_SUSTAIN_ID,
+            value: if on { MAX_LEVEL } else { 0 },
+        }),
+    }
+}
+
+pub fn set_damper(id_seq: &IdSeq, track: &Track, range: &Range<Time>, on: bool) -> AppliedCommand {
+    let mut patch = vec![];
+    let on_before = is_cc_switch_on(cc_value_at(&track.events, &range.0, &MIDI_CC_SUSTAIN_ID));
+    let on_after = is_cc_switch_on(cc_value_at(
+        &track.events,
+        &(range.1 + 1),
+        &MIDI_CC_SUSTAIN_ID,
+    ));
+
+    clear_cc_events(track, range, MIDI_CC_SUSTAIN_ID, &mut patch);
+    if on {
+        if !on_before {
+            let on_ev = sustain_event(&id_seq, &range.0, true);
+            patch.push(EventAction::Insert(on_ev));
+        }
+        if !on_after {
+            let off_ev = sustain_event(&id_seq, &range.1, false);
+            patch.push(EventAction::Insert(off_ev));
+        }
+    } else {
+        if on_before {
+            let off_ev = sustain_event(&id_seq, &range.0, false);
+            patch.push(EventAction::Insert(off_ev));
+        }
+        if on_after {
+            let on_ev = sustain_event(&id_seq, &range.1, true);
+            patch.push(EventAction::Insert(on_ev));
+        }
+    }
+
+    (
+        EditCommandId::SetDamper,
+        vec![CommandDiff::Changeset { patch }],
+    )
+}
+
+fn clear_cc_events(
+    track: &Track,
+    range: &Range<Time>,
+    cc_id: ControllerId,
+    patch: &mut Vec<EventAction>,
+) {
+    for ev in &track.events {
+        if range_contains(range, ev.at) {
+            if let TrackEventType::Controller(cc) = &ev.event {
+                if cc.controller_id == cc_id {
+                    patch.push(EventAction::Delete(ev.clone()));
+                }
+            }
+        }
+    }
+}
+
+fn cc_value_at(events: &Vec<TrackEvent>, at: &Time, cc_id: &ControllerId) -> Level {
+    let mut idx = events.partition_point(|x| x.at < *at);
+    while idx > 0 {
+        idx -= 1;
+        if let Some(ev) = events.get(idx) {
+            if let TrackEventType::Controller(cc) = &ev.event {
+                if cc.controller_id == *cc_id {
+                    return cc.value;
+                }
+            }
+        }
+    }
+    return 0; // default
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn make_test_track() -> Track {
+        let mut events: Vec<TrackEvent> = vec![];
+        events.push(TrackEvent {
+            id: 10,
+            at: 10,
+            event: TrackEventType::Controller(ControllerSetValue {
+                controller_id: 13,
+                value: 55,
+            }),
+        });
+        events.push(TrackEvent {
+            id: 20,
+            at: 14,
+            event: TrackEventType::Note(Note {
+                pitch: 10,
+                velocity: 20,
+                duration: 30,
+            }),
+        });
+        events.push(TrackEvent {
+            id: 30,
+            at: 15,
+            event: TrackEventType::Controller(ControllerSetValue {
+                controller_id: 44,
+                value: 60,
+            }),
+        });
+        events.push(TrackEvent {
+            id: 40,
+            at: 20,
+            event: TrackEventType::Controller(ControllerSetValue {
+                controller_id: 13,
+                value: 66,
+            }),
+        });
+        let mut track = Track::default();
+        track.events = events;
+        track
+    }
+
+    #[test]
+    fn check_cc_value_at() {
+        let track = make_test_track();
+
+        assert_eq!(55, cc_value_at(&track.events, &20, &13));
+        assert_eq!(66, cc_value_at(&track.events, &21, &13));
+        assert_eq!(60, cc_value_at(&track.events, &21, &44));
+        assert_eq!(0, cc_value_at(&track.events, &21, &99));
+        assert_eq!(0, cc_value_at(&track.events, &0, &99));
+    }
+
+    #[test]
+    fn check_set_damper_to() {
+        let mut track = make_test_track();
+        let id_seq = IdSeq::new(0);
+        let applied_command = set_damper(&id_seq, &mut track, &(13, 17), true);
+        let mut changeset = Changeset::empty();
+        into_changeset(&track, &applied_command.1, &mut changeset);
+        track.patch(&changeset);
+
+        let expected_ids: Vec<EventId> = vec![10, 0, 20, 30, 1, 40];
+        assert_eq!(
+            expected_ids,
+            track
+                .events
+                .iter()
+                .map(|ev| ev.id)
+                .collect::<Vec<EventId>>()
+        );
+
+        let expected_states: Vec<Option<bool>> = vec![
+            Some(false),
+            Some(true),
+            None,
+            Some(false),
+            Some(false),
+            Some(true),
+        ];
+        assert_eq!(
+            expected_states,
+            track
+                .events
+                .iter()
+                .map(|ev| if let TrackEventType::Controller(ctl) = &ev.event {
+                    Some(is_cc_switch_on(ctl.value))
+                } else {
+                    None
+                })
+                .collect::<Vec<Option<bool>>>()
+        );
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index b291dd6..1d7b62a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -11,6 +11,7 @@ mod audio_setup;
 mod changeset;
 mod common;
 mod config;
+mod edit_commands;
 mod engine;
 mod midi;
 mod midi_vst;
diff --git a/src/stave.rs b/src/stave.rs
index d6c60a5..8540c90 100644
--- a/src/stave.rs
+++ b/src/stave.rs
@@ -11,23 +11,24 @@ use egui::Rgba;
 use ordered_float::OrderedFloat;
 use serde::{Deserialize, Serialize};
 
-use crate::changeset::{Changeset, EventAction};
 use crate::common::{Time, VersionId};
-use crate::track::{
-    EventId, Level, Note, Pitch, Track, TrackEvent, TrackEventType, MIDI_CC_SUSTAIN_ID,
+use crate::edit_commands::{
+    accent_selected_notes, add_new_note, delete_selected, set_damper, shift_selected, shift_tail,
+    stretch_selected_notes, tape_delete, tape_insert, transpose_selected_notes,
 };
-use crate::track_edit::{
-    add_note, delete_events, edit_events, set_damper_to, shift_events, shift_tail, tape_cut,
-    tape_insert, TimeSelection,
+use crate::track::{
+    export_smf, EventId, Level, Pitch, Track, TrackEvent, TrackEventType, MIDI_CC_SUSTAIN_ID,
 };
-use crate::track_history::{ActionId, TrackHistory};
+use crate::track_edit::TimeSelection;
+use crate::track_history::TrackHistory;
 use crate::{util, Pix};
 
 // Tone 60 is C3, tones start at C-2 (21).
 const PIANO_LOWEST_KEY: Pitch = 21;
 const PIANO_KEY_COUNT: Pitch = 88;
 const PIANO_DAMPER_LINE: Pitch = PIANO_LOWEST_KEY - 1;
-const PIANO_KEY_LINES: Range<Pitch> = PIANO_LOWEST_KEY..(PIANO_LOWEST_KEY + PIANO_KEY_COUNT);
+pub(crate) const PIANO_KEY_LINES: Range<Pitch> =
+    PIANO_LOWEST_KEY..(PIANO_LOWEST_KEY + PIANO_KEY_COUNT);
 // Lines including controller values placeholder.
 const STAVE_KEY_LINES: Range<Pitch> = (PIANO_LOWEST_KEY - 1)..(PIANO_LOWEST_KEY + PIANO_KEY_COUNT);
 
@@ -179,12 +180,8 @@ impl Stave {
     }
 
     pub fn save_to(&mut self, file_path: &PathBuf) {
-        self.history.with_track(|track| track.export_smf(file_path));
-    }
-
-    pub fn load_from(&mut self, file_path: &PathBuf) {
         self.history
-            .update_track(None, |track, _| track.import_smf(file_path));
+            .with_track(|track| export_smf(&track.events, file_path));
     }
 
     /// Pixel/uSec, can be cached.
@@ -435,13 +432,17 @@ impl Stave {
                 egui::Key::Delete,
             ))
         }) {
-            self.history.update_track(None, |track, changeset| {
-                if let Some(time_selection) = &self.time_selection {
-                    tape_cut(track, changeset, &time_selection);
-                }
-                // Deleting both time and event selection in one command for convenience, these can be separate.
-                delete_events(track, changeset, &self.note_selection.selected);
-            });
+            if let Some(time_selection) = &self.time_selection {
+                self.history.update_track(|track| {
+                    tape_delete(track, &(time_selection.from, time_selection.to))
+                });
+            }
+            if !self.note_selection.selected.is_empty() {
+                self.history.update_track(|track| {
+                    // Deleting both time and event selection in one command for convenience, these can be separate.
+                    delete_selected(track, &self.note_selection.selected)
+                });
+            }
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(
@@ -449,11 +450,10 @@ impl Stave {
                 egui::Key::Insert,
             ))
         }) {
-            self.history.update_track(None, |track, changeset| {
-                if let Some(time_selection) = &self.time_selection {
-                    tape_insert(track, changeset, &time_selection);
-                }
-            });
+            if let Some(time_selection) = &self.time_selection {
+                self.history
+                    .update_track(|_track| tape_insert(&(time_selection.from, time_selection.to)));
+            }
         }
 
         // Tail shift
@@ -463,15 +463,9 @@ impl Stave {
                 egui::Key::ArrowRight,
             ))
         }) {
-            self.history
-                .update_track(Some("tail_shift_right"), |track, changeset| {
-                    shift_tail(
-                        track,
-                        changeset,
-                        &(self.cursor_position),
-                        Stave::KEYBOARD_TIME_STEP,
-                    );
-                });
+            self.history.update_track(|_track| {
+                shift_tail(&(self.cursor_position), &Stave::KEYBOARD_TIME_STEP)
+            });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(
@@ -479,15 +473,9 @@ impl Stave {
                 egui::Key::ArrowLeft,
             ))
         }) {
-            self.history
-                .update_track(Some("tail_shift_left"), |track, changeset| {
-                    shift_tail(
-                        track,
-                        changeset,
-                        &(self.cursor_position),
-                        -Stave::KEYBOARD_TIME_STEP,
-                    );
-                });
+            self.history.update_track(|_track| {
+                shift_tail(&(self.cursor_position), &-Stave::KEYBOARD_TIME_STEP)
+            });
         }
 
         // Note time moves
@@ -497,15 +485,13 @@ impl Stave {
                 egui::Key::ArrowRight,
             )) || i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::SHIFT, egui::Key::L))
         }) {
-            self.history
-                .update_track(Some("note_shift_right"), |track, changeset| {
-                    shift_events(
-                        track,
-                        changeset,
-                        &(|ev| self.note_selection.contains(ev)),
-                        Stave::KEYBOARD_TIME_STEP,
-                    );
-                });
+            self.history.update_track(|track| {
+                shift_selected(
+                    track,
+                    &self.note_selection.selected,
+                    &Stave::KEYBOARD_TIME_STEP,
+                )
+            });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(
@@ -513,80 +499,64 @@ impl Stave {
                 egui::Key::ArrowLeft,
             )) || i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::SHIFT, egui::Key::H))
         }) {
-            self.history
-                .update_track(Some("note_shift_left"), |track, changeset| {
-                    shift_events(
-                        track,
-                        changeset,
-                        &(|ev| self.note_selection.contains(ev)),
-                        -Stave::KEYBOARD_TIME_STEP,
-                    );
-                });
+            self.history.update_track(|track| {
+                shift_selected(
+                    track,
+                    &self.note_selection.selected,
+                    &-Stave::KEYBOARD_TIME_STEP,
+                )
+            });
         }
 
         // Note edits
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::NONE, egui::Key::H))
         }) {
-            self.edit_selected_notes(Some("note_duration_increase"), |note| {
-                let mut note = note.clone();
-                note.duration = note
-                    .duration
-                    .checked_sub(Stave::KEYBOARD_TIME_STEP)
-                    .unwrap_or(0);
-                Some(note)
+            self.history.update_track(|track| {
+                stretch_selected_notes(
+                    track,
+                    &self.note_selection.selected,
+                    &-Stave::KEYBOARD_TIME_STEP,
+                )
             });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::NONE, egui::Key::L))
         }) {
-            self.edit_selected_notes(Some("note_duration_decrease"), |note| {
-                let mut note = note.clone();
-                note.duration = note
-                    .duration
-                    .checked_add(Stave::KEYBOARD_TIME_STEP)
-                    .unwrap_or(0);
-                Some(note)
+            self.history.update_track(|track| {
+                stretch_selected_notes(
+                    track,
+                    &self.note_selection.selected,
+                    &Stave::KEYBOARD_TIME_STEP,
+                )
             });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::NONE, egui::Key::U))
         }) {
-            self.edit_selected_notes(Some("note_pitch_up"), |note| {
-                let mut note = note.clone();
-                if PIANO_KEY_LINES.contains(&(note.pitch + 1)) {
-                    note.pitch += 1;
-                }
-                Some(note)
+            self.history.update_track(|track| {
+                transpose_selected_notes(track, &self.note_selection.selected, 1)
             });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::NONE, egui::Key::J))
         }) {
-            self.edit_selected_notes(Some("note_pitch_down"), |note| {
-                let mut note = note.clone();
-                if PIANO_KEY_LINES.contains(&(note.pitch - 1)) {
-                    note.pitch -= 1;
-                }
-                Some(note)
+            self.history.update_track(|track| {
+                transpose_selected_notes(track, &self.note_selection.selected, -1)
             });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::NONE, egui::Key::I))
         }) {
-            self.edit_selected_notes(Some("note_velocity_increase"), |note| {
-                let mut note = note.clone();
-                note.velocity = note.velocity.checked_add(1).unwrap_or(Level::MAX);
-                Some(note)
+            self.history.update_track(|track| {
+                accent_selected_notes(track, &self.note_selection.selected, 1)
             });
         }
         if response.ctx.input_mut(|i| {
             i.consume_shortcut(&egui::KeyboardShortcut::new(Modifiers::NONE, egui::Key::K))
         }) {
-            self.edit_selected_notes(Some("note_velocity_decrease"), |note| {
-                let mut note = note.clone();
-                note.velocity = note.velocity.checked_sub(1).unwrap_or(Level::MIN);
-                Some(note)
+            self.history.update_track(|track| {
+                accent_selected_notes(track, &self.note_selection.selected, -1)
             });
         }
 
@@ -666,36 +636,6 @@ impl Stave {
         self.history.with_track(|track| track.max_time())
     }
 
-    pub fn edit_selected_notes<Action: Fn(&Note) -> Option<Note> + 'static>(
-        &mut self,
-        action_id: ActionId,
-        action: Action,
-    ) {
-        // Adapt note action as event action.
-        let event_action = move |ev: &TrackEvent| {
-            if let TrackEvent {
-                event: TrackEventType::Note(n),
-                ..
-            } = &ev
-            {
-                if let Some(n) = action(n) {
-                    let mut ev = ev.clone();
-                    ev.event = TrackEventType::Note(n);
-                    return Some(EventAction::Update(ev.clone(), ev));
-                }
-            }
-            None
-        };
-        self.history.update_track(action_id, |track, changeset| {
-            edit_events(
-                track,
-                changeset,
-                &|ev| self.note_selection.contains(ev),
-                &event_action,
-            );
-        });
-    }
-
     fn update_time_selection(&mut self, response: &egui::Response, time: &Option<Time>) {
         let drag_button = PointerButton::Primary;
         if response.clicked_by(drag_button) {
@@ -747,20 +687,18 @@ impl Stave {
             if let Some(draw) = &mut self.note_draw {
                 if !draw.time.is_empty() {
                     let time_range = (draw.time.from, draw.time.to);
-                    self.history.update_track(
-                        None,
-                        |track: &mut Track, changeset: &mut Changeset| {
-                            if draw.pitch == PIANO_DAMPER_LINE {
-                                if modifiers.alt {
-                                    set_damper_to(track, changeset, time_range, false);
-                                } else {
-                                    set_damper_to(track, changeset, time_range, true);
-                                }
+                    let id_seq = &self.history.id_seq.clone();
+                    self.history.update_track(|track: &Track| {
+                        if draw.pitch == PIANO_DAMPER_LINE {
+                            if modifiers.alt {
+                                set_damper(id_seq, track, &time_range, false)
                             } else {
-                                add_note(track, changeset, time_range, draw.pitch, 64);
+                                set_damper(id_seq, track, &time_range, true)
                             }
-                        },
-                    );
+                        } else {
+                            add_new_note(id_seq, &time_range, &draw.pitch)
+                        }
+                    });
                 }
             }
             self.note_draw = None;
diff --git a/src/track.rs b/src/track.rs
index 5db32fb..1f028ce 100644
--- a/src/track.rs
+++ b/src/track.rs
@@ -30,6 +30,11 @@ pub fn is_cc_switch_on(x: Level) -> bool {
     x >= 64
 }
 
+pub enum TrackLane {
+    Note(Pitch),
+    Controller(ControllerId),
+}
+
 #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
 pub struct Note {
     pub pitch: Pitch,
@@ -45,6 +50,7 @@ pub struct ControllerSetValue {
 
 #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
 pub enum TrackEventType {
+    // TODO (cleanup) maybe move Note and ControllerSetValue here (use Note{...} without the extra type).
     Note(Note),
     Controller(ControllerSetValue),
 }
@@ -84,27 +90,9 @@ pub struct Track {
     /* Events should always be kept ordered by start time ascending.
     This is a requirement of TrackSource. */
     pub events: Vec<TrackEvent>,
-    pub id_seq: IdSeq,
 }
 
 impl Track {
-    pub fn import_smf(&mut self, file_path: &PathBuf) {
-        let data = std::fs::read(&file_path).unwrap();
-        let events = midi::load_smf(&data);
-        self.events = from_midi_events(&mut self.id_seq, events.0, events.1 as Time);
-        self.commit();
-    }
-
-    pub fn export_smf(&self, file_path: &PathBuf) {
-        let usec_per_tick = 26u32;
-        let midi_events = to_midi_events(&self.events, usec_per_tick);
-        let mut binary = Vec::new();
-        midi::serialize_smf(midi_events, usec_per_tick, &mut binary)
-            .expect("Cannot store SMF track.");
-        std::fs::write(&file_path, binary)
-            .expect(&*format!("Cannot save to {}", &file_path.display()));
-    }
-
     pub fn reset(&mut self, snapshot: Snapshot) {
         self.events = snapshot.events;
     }
@@ -123,7 +111,7 @@ impl Track {
     }
 
     pub fn patch(&mut self, changeset: &Changeset) {
-        // Can this be shorter? See also [revert]
+        // TODO (DRY) See also [revert]
         let mut track_map = self.index_events();
         for ea in changeset.changes.values() {
             match ea.after() {
@@ -184,7 +172,7 @@ impl Track {
 }
 
 pub fn from_midi_events(
-    id_seq: &mut IdSeq,
+    id_seq: &IdSeq,
     events: Vec<midly::TrackEvent<'static>>,
     tick_duration: Time,
 ) -> Vec<TrackEvent> {
@@ -235,6 +223,20 @@ pub fn from_midi_events(
     track_events
 }
 
+pub fn import_smf(id_seq: &IdSeq, file_path: &PathBuf) -> Vec<TrackEvent> {
+    let data = std::fs::read(&file_path).unwrap();
+    let events = midi::load_smf(&data);
+    from_midi_events(&id_seq, events.0, events.1 as Time)
+}
+
+pub fn export_smf(events: &Vec<TrackEvent>, file_path: &PathBuf) {
+    let usec_per_tick = 26u32;
+    let midi_events = to_midi_events(&events, usec_per_tick);
+    let mut binary = Vec::new();
+    midi::serialize_smf(midi_events, usec_per_tick, &mut binary).expect("Cannot store SMF track.");
+    std::fs::write(&file_path, binary).expect(&*format!("Cannot save to {}", &file_path.display()));
+}
+
 /// Reverse of from_midi_events
 pub fn to_midi_events(
     events: &Vec<TrackEvent>,
@@ -299,24 +301,18 @@ mod tests {
 
     #[test]
     fn track_load() {
-        let mut track = Track::default();
-        assert!(track.events.is_empty());
-
-        let path = PathBuf::from("./target/test_track_load.mid");
-        track.export_smf(&path);
-        track.import_smf(&path);
-        assert!(track.events.is_empty());
-
-        let short = PathBuf::from("./test/files/short.mid");
-        track.import_smf(&short);
-        assert_eq!(track.events.len(), 10);
-        track.export_smf(&path);
+        let id_seq = IdSeq::new(0);
+        let path_short = PathBuf::from("./test/files/short.mid");
+        let events = import_smf(&id_seq, &path_short);
+        assert_eq!(events.len(), 10);
+        let path_exported = PathBuf::from("./target/test_track_load.mid");
+        export_smf(&events, &path_exported);
 
         // The recorded SMD may have some additional system/heartbeat events,
         // so comparing the sequence only after a save.
-        let mut track_loaded = Track::default();
-        track_loaded.import_smf(&path);
-        assert_eq!(track_loaded.events.len(), 10);
-        assert_eq!(track.events, track_loaded.events);
+        let id_seq = IdSeq::new(0);
+        let events2 = import_smf(&id_seq, &path_exported);
+        assert_eq!(events2.len(), 10);
+        assert_eq!(events, events2);
     }
 }
diff --git a/src/track_edit.rs b/src/track_edit.rs
index f63a720..64519cb 100644
--- a/src/track_edit.rs
+++ b/src/track_edit.rs
@@ -1,16 +1,16 @@
-// TODO (cleanup) All functions in this module should accept only immutable track.
 //   It is possible to not use id generator here, by having some id-placeholder value.
 
-use std::collections::HashSet;
-
-use crate::changeset::{Changeset, EventAction, EventFn};
+// TODO (cleanup) Remove unused parts from here. Most of the functions here are replaced by edit_commands.
 use crate::common::Time;
-use crate::track::{
-    is_cc_switch_on, ControllerId, ControllerSetValue, EventId, Level, Note, Pitch, Track,
-    TrackEvent, TrackEventType, MAX_LEVEL, MIDI_CC_SUSTAIN_ID,
-};
-use crate::util;
-use crate::util::{range_contains, IdSeq};
+
+// use std::collections::HashSet;
+// use crate::changeset::{Changeset, EventAction, EventFn};
+// use crate::track::{
+//     is_cc_switch_on, ControllerId, ControllerSetValue, EventId, Level, Note, Pitch, Track,
+//     TrackEvent, TrackEventType, MAX_LEVEL, MIDI_CC_SUSTAIN_ID,
+// };
+// use crate::util;
+// use crate::util::{range_contains, IdSeq};
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct TimeSelection {
@@ -40,264 +40,169 @@ impl TimeSelection {
     }
 }
 
-pub fn add_note(
-    track: &mut Track,
-    changeset: &mut Changeset,
-    time_range: (Time, Time),
-    pitch: Pitch,
-    level: Level,
-) {
-    changeset.add(EventAction::Insert(TrackEvent {
-        id: track.id_seq.next(),
-        at: time_range.0,
-        event: TrackEventType::Note(Note {
-            pitch,
-            velocity: level,
-            duration: time_range.1 - time_range.0,
-        }),
-    }));
-}
-
-pub fn edit_events<Selector: Fn(&TrackEvent) -> bool>(
-    track: &mut Track,
-    changeset: &mut Changeset,
-    selector: &Selector,
-    action: &EventFn,
-) {
-    for ev in &mut track.events {
-        if selector(&ev) {
-            if let Some(a) = action(&ev) {
-                changeset.add(a);
-            }
-        }
-    }
-}
-
-pub fn delete_events(track: &mut Track, changeset: &mut Changeset, event_ids: &HashSet<EventId>) {
-    edit_events(track, changeset, &|ev| event_ids.contains(&ev.id), &|ev| {
-        Some(EventAction::Delete(ev.clone()))
-    });
-}
-
-pub fn tape_cut(track: &mut Track, changeset: &mut Changeset, time_selection: &TimeSelection) {
-    dbg!("tape_cut", time_selection);
-    edit_events(
-        track,
-        changeset,
-        &|ev| time_selection.contains(ev.at),
-        &|ev| Some(EventAction::Delete(ev.clone())),
-    );
-    shift_events(
-        track,
-        changeset,
-        &|ev| time_selection.before(ev.at),
-        -(time_selection.length() as i64),
-    );
-}
-
-pub fn shift_events<Pred: Fn(&TrackEvent) -> bool>(
-    track: &mut Track,
-    changeset: &mut Changeset,
-    selector: &Pred,
-    d: i64,
-) {
-    edit_events(track, changeset, selector, &move |ev| {
-        let mut nev = ev.clone();
-        nev.at += d;
-        Some(EventAction::Update(ev.clone(), nev))
-    });
-}
-
-pub fn tape_insert(track: &mut Track, changeset: &mut Changeset, time_selection: &TimeSelection) {
-    dbg!("tape_insert", time_selection);
-    shift_events(
-        track,
-        changeset,
-        &|ev| time_selection.after_start(ev.at),
-        time_selection.length() as i64,
-    );
-}
-
-pub fn shift_tail(track: &mut Track, changeset: &mut Changeset, at: &Time, dt: i64) {
-    dbg!("tail_shift", at, dt);
-    shift_events(track, changeset, &|ev| &ev.at >= at, dt);
-}
-
-fn clear_cc_events(
-    track: &mut Track,
-    changeset: &mut Changeset,
-    time_range: util::Range<Time>,
-    cc_id: ControllerId,
-) {
-    for ev in &track.events {
-        if range_contains(time_range, ev.at) {
-            if let TrackEventType::Controller(cc) = &ev.event {
-                if cc.controller_id == cc_id {
-                    changeset.add(EventAction::Delete(ev.clone()));
-                }
-            }
-        }
-    }
-}
-
-fn sustain_event(id_seq: &mut IdSeq, at: &Time, on: bool) -> TrackEvent {
-    TrackEvent {
-        id: id_seq.next(),
-        at: *at,
-        event: TrackEventType::Controller(ControllerSetValue {
-            controller_id: MIDI_CC_SUSTAIN_ID,
-            value: if on { MAX_LEVEL } else { 0 },
-        }),
-    }
-}
-
-pub fn set_damper_to(
-    track: &mut Track,
-    changeset: &mut Changeset,
-    time_range: util::Range<Time>,
-    on: bool,
-) {
-    dbg!("set_damper", time_range, on);
-    let on_before = is_cc_switch_on(cc_value_at(
-        &track.events,
-        &time_range.0,
-        &MIDI_CC_SUSTAIN_ID,
-    ));
-    let on_after = is_cc_switch_on(cc_value_at(
-        &track.events,
-        &time_range.1,
-        &MIDI_CC_SUSTAIN_ID,
-    ));
-
-    clear_cc_events(track, changeset, time_range, MIDI_CC_SUSTAIN_ID);
-
-    if on {
-        if !on_before {
-            let on_ev = sustain_event(&mut track.id_seq, &time_range.0, true);
-            changeset.add(EventAction::Insert(on_ev));
-        }
-        if !on_after {
-            let off_ev = sustain_event(&mut track.id_seq, &time_range.1, false);
-            changeset.add(EventAction::Insert(off_ev));
-        }
-    } else {
-        if on_before {
-            let off_ev = sustain_event(&mut track.id_seq, &time_range.0, false);
-            changeset.add(EventAction::Insert(off_ev));
-        }
-        if on_after {
-            let on_ev = sustain_event(&mut track.id_seq, &time_range.1, true);
-            changeset.add(EventAction::Insert(on_ev));
-        }
-    }
-}
-
-fn cc_value_at(events: &Vec<TrackEvent>, at: &Time, cc_id: &ControllerId) -> Level {
-    let mut idx = events.partition_point(|x| x.at < *at);
-    while idx > 0 {
-        idx -= 1;
-        if let Some(ev) = events.get(idx) {
-            if let TrackEventType::Controller(cc) = &ev.event {
-                if cc.controller_id == *cc_id {
-                    return cc.value;
-                }
-            }
-        }
-    }
-    return 0; // default
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    fn make_test_track() -> Track {
-        let mut events: Vec<TrackEvent> = vec![];
-        events.push(TrackEvent {
-            id: 10,
-            at: 10,
-            event: TrackEventType::Controller(ControllerSetValue {
-                controller_id: 13,
-                value: 55,
-            }),
-        });
-        events.push(TrackEvent {
-            id: 20,
-            at: 14,
-            event: TrackEventType::Note(Note {
-                pitch: 10,
-                velocity: 20,
-                duration: 30,
-            }),
-        });
-        events.push(TrackEvent {
-            id: 30,
-            at: 15,
-            event: TrackEventType::Controller(ControllerSetValue {
-                controller_id: 44,
-                value: 60,
-            }),
-        });
-        events.push(TrackEvent {
-            id: 40,
-            at: 20,
-            event: TrackEventType::Controller(ControllerSetValue {
-                controller_id: 13,
-                value: 66,
-            }),
-        });
-        let mut track = Track::default();
-        track.events = events;
-        track
-    }
-
-    #[test]
-    fn check_cc_value_at() {
-        let track = make_test_track();
-
-        assert_eq!(55, cc_value_at(&track.events, &20, &13));
-        assert_eq!(66, cc_value_at(&track.events, &21, &13));
-        assert_eq!(60, cc_value_at(&track.events, &21, &44));
-        assert_eq!(0, cc_value_at(&track.events, &21, &99));
-        assert_eq!(0, cc_value_at(&track.events, &0, &99));
-    }
-
-    #[test]
-    fn check_set_damper_to() {
-        let mut track = make_test_track();
-        let mut changeset = Changeset::empty();
-        set_damper_to(&mut track, &mut changeset, (13, 17), true);
-        track.patch(&changeset);
-
-        let expected_ids: Vec<EventId> = vec![10, 0, 20, 30, 1, 40];
-        assert_eq!(
-            expected_ids,
-            track
-                .events
-                .iter()
-                .map(|ev| ev.id)
-                .collect::<Vec<EventId>>()
-        );
-
-        let expected_states: Vec<Option<bool>> = vec![
-            Some(false),
-            Some(true),
-            None,
-            Some(false),
-            Some(false),
-            Some(true),
-        ];
-        assert_eq!(
-            expected_states,
-            track
-                .events
-                .iter()
-                .map(|ev| if let TrackEventType::Controller(ctl) = &ev.event {
-                    Some(is_cc_switch_on(ctl.value))
-                } else {
-                    None
-                })
-                .collect::<Vec<Option<bool>>>()
-        );
-    }
-}
+// pub fn add_note(
+//     track: &mut Track,
+//     changeset: &mut Changeset,
+//     time_range: (Time, Time),
+//     pitch: Pitch,
+//     level: Level,
+// ) {
+//     changeset.add(EventAction::Insert(TrackEvent {
+//         id: track.id_seq.borrow_mut().next(),
+//         at: time_range.0,
+//         event: TrackEventType::Note(Note {
+//             pitch,
+//             velocity: level,
+//             duration: time_range.1 - time_range.0,
+//         }),
+//     }));
+// }
+//
+// pub fn edit_events<Selector: Fn(&TrackEvent) -> bool>(
+//     track: &mut Track,
+//     changeset: &mut Changeset,
+//     selector: &Selector,
+//     action: &EventFn,
+// ) {
+//     for ev in &mut track.events {
+//         if selector(&ev) {
+//             if let Some(a) = action(&ev) {
+//                 changeset.add(a);
+//             }
+//         }
+//     }
+// }
+//
+// pub fn delete_events(track: &mut Track, changeset: &mut Changeset, event_ids: &HashSet<EventId>) {
+//     edit_events(track, changeset, &|ev| event_ids.contains(&ev.id), &|ev| {
+//         Some(EventAction::Delete(ev.clone()))
+//     });
+// }
+//
+// pub fn tape_cut(track: &mut Track, changeset: &mut Changeset, time_selection: &TimeSelection) {
+//     dbg!("tape_cut", time_selection);
+//     edit_events(
+//         track,
+//         changeset,
+//         &|ev| time_selection.contains(ev.at),
+//         &|ev| Some(EventAction::Delete(ev.clone())),
+//     );
+//     shift_events(
+//         track,
+//         changeset,
+//         &|ev| time_selection.before(ev.at),
+//         -(time_selection.length() as i64),
+//     );
+// }
+//
+// pub fn shift_events<Pred: Fn(&TrackEvent) -> bool>(
+//     track: &mut Track,
+//     changeset: &mut Changeset,
+//     selector: &Pred,
+//     d: i64,
+// ) {
+//     edit_events(track, changeset, selector, &move |ev| {
+//         let mut nev = ev.clone();
+//         nev.at += d;
+//         Some(EventAction::Update(ev.clone(), nev))
+//     });
+// }
+//
+// pub fn tape_insert(track: &mut Track, changeset: &mut Changeset, time_selection: &TimeSelection) {
+//     dbg!("tape_insert", time_selection);
+//     shift_events(
+//         track,
+//         changeset,
+//         &|ev| time_selection.after_start(ev.at),
+//         time_selection.length() as i64,
+//     );
+// }
+//
+// pub fn shift_tail(track: &mut Track, changeset: &mut Changeset, at: &Time, dt: i64) {
+//     dbg!("tail_shift", at, dt);
+//     shift_events(track, changeset, &|ev| &ev.at >= at, dt);
+// }
+//
+// fn clear_cc_events(
+//     track: &mut Track,
+//     changeset: &mut Changeset,
+//     time_range: util::Range<Time>,
+//     cc_id: ControllerId,
+// ) {
+//     for ev in &track.events {
+//         if range_contains(&time_range, ev.at) {
+//             if let TrackEventType::Controller(cc) = &ev.event {
+//                 if cc.controller_id == cc_id {
+//                     changeset.add(EventAction::Delete(ev.clone()));
+//                 }
+//             }
+//         }
+//     }
+// }
+//
+// fn sustain_event(id_seq: &mut IdSeq, at: &Time, on: bool) -> TrackEvent {
+//     TrackEvent {
+//         id: id_seq.next(),
+//         at: *at,
+//         event: TrackEventType::Controller(ControllerSetValue {
+//             controller_id: MIDI_CC_SUSTAIN_ID,
+//             value: if on { MAX_LEVEL } else { 0 },
+//         }),
+//     }
+// }
+//
+// pub fn set_damper_to(
+//     track: &mut Track,
+//     changeset: &mut Changeset,
+//     time_range: util::Range<Time>,
+//     on: bool,
+// ) {
+//     dbg!("set_damper", time_range, on);
+//     let on_before = is_cc_switch_on(cc_value_at(
+//         &track.events,
+//         &time_range.0,
+//         &MIDI_CC_SUSTAIN_ID,
+//     ));
+//     let on_after = is_cc_switch_on(cc_value_at(
+//         &track.events,
+//         &time_range.1,
+//         &MIDI_CC_SUSTAIN_ID,
+//     ));
+//
+//     clear_cc_events(track, changeset, time_range, MIDI_CC_SUSTAIN_ID);
+//
+//     if on {
+//         if !on_before {
+//             let on_ev = sustain_event(&mut track.id_seq, &time_range.0, true);
+//             changeset.add(EventAction::Insert(on_ev));
+//         }
+//         if !on_after {
+//             let off_ev = sustain_event(&mut track.id_seq, &time_range.1, false);
+//             changeset.add(EventAction::Insert(off_ev));
+//         }
+//     } else {
+//         if on_before {
+//             let off_ev = sustain_event(&mut track.id_seq, &time_range.0, false);
+//             changeset.add(EventAction::Insert(off_ev));
+//         }
+//         if on_after {
+//             let on_ev = sustain_event(&mut track.id_seq, &time_range.1, true);
+//             changeset.add(EventAction::Insert(on_ev));
+//         }
+//     }
+// }
+//
+// fn cc_value_at(events: &Vec<TrackEvent>, at: &Time, cc_id: &ControllerId) -> Level {
+//     let mut idx = events.partition_point(|x| x.at < *at);
+//     while idx > 0 {
+//         idx -= 1;
+//         if let Some(ev) = events.get(idx) {
+//             if let TrackEventType::Controller(cc) = &ev.event {
+//                 if cc.controller_id == *cc_id {
+//                     return cc.value;
+//                 }
+//             }
+//         }
+//     }
+//     return 0; // default
+// }
diff --git a/src/track_history.rs b/src/track_history.rs
index 7f4a330..b86a61b 100644
--- a/src/track_history.rs
+++ b/src/track_history.rs
@@ -1,24 +1,24 @@
+use std::cell::RefCell;
+use std::fs;
 use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
 use std::time::{Duration, Instant};
-use std::{fs, mem};
 
 use glob::glob;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
 
-use crate::changeset::{Changeset, Patch, Snapshot};
+use crate::changeset::{Changeset, EventAction, HistoryLogEntry, Snapshot};
 use crate::common::VersionId;
-use crate::track::Track;
+use crate::edit_commands::{into_changeset, revert_diffs, CommandDiff, EditCommandId};
+use crate::track::{import_smf, Track};
 use crate::util;
 use crate::util::IdSeq;
 
-pub type ActionId = Option<&'static str>;
-
 #[derive(Debug)]
 struct ActionThrottle {
     timestamp: Instant,
-    action_id: ActionId,
+    // FIXME action_id: ActionId,
     changeset: Changeset,
 }
 
@@ -31,8 +31,8 @@ impl ActionThrottle {
 // Undo/redo history and snapshots.
 #[derive(Debug)]
 pub struct TrackHistory {
-    /// Normally should not be used from outside. Made it pub as double-borrow workaround.
     pub track: Arc<RwLock<Track>>,
+    pub id_seq: Arc<IdSeq>,
     pub version: VersionId,
     pub max_version: VersionId, // May be higher than self.version after an undo.
     pub directory: PathBuf,
@@ -61,24 +61,25 @@ impl TrackHistory {
         action(&track)
     }
 
-    pub fn update_track<Action: FnOnce(&mut Track, &mut Changeset)>(
+    pub fn update_track<Action: FnOnce(&Track) -> (EditCommandId, Vec<CommandDiff>)>(
         &mut self,
-        action_id: ActionId,
         action: Action,
     ) {
-        let changeset = {
+        let applied_command = {
             let mut track = self.track.write().expect("Write to track.");
+            let applied_command = action(&track);
             let mut changeset = Changeset::empty();
-            action(&mut track, &mut changeset);
+            into_changeset(&track, &applied_command.1, &mut changeset);
             track.patch(&changeset);
             track.commit();
-            changeset
+            applied_command
         };
 
-        // TODO Reimplement throttling (batch sequences of repeatable
-        //   heavy operations like tail_shift).
-        dbg!(action_id, changeset.changes.len());
-        self.update(changeset);
+        // TODO (tokio, throttle, diff history) Reimplement throttling (batch sequences of repeatable
+        //   heavy operations). Applying immediately for now.
+
+        self.update(applied_command);
+
         // let now = Instant::now();
         // if let Some(throttle) = &mut self.throttle {
         //     if action_id.is_some() && throttle.action_id == action_id && throttle.is_waiting(now) {
@@ -97,29 +98,48 @@ impl TrackHistory {
         // }
     }
 
-    pub fn do_pending(&mut self) {
-        // This is ugly, just making it work for now. History may need some scheduled events.
-        // To do this asynchronously one would need to put id behind an Arc<RwLock<>> or
-        // use Tokio here (and in the engine).
-        if let Some(throttle) = &self.throttle {
-            if !throttle.is_waiting(Instant::now()) {
-                let throttle = mem::replace(&mut self.throttle, None).unwrap();
-                self.update(throttle.changeset);
-                self.throttle = None;
-            }
-        }
-    }
-
-    /// Normally should not be used from outside. Made it pub as double-borrow workaround.
-    fn update(&mut self, changeset: Changeset) {
-        if changeset.changes.is_empty() {
+    // TODO (tokio, throttle, diff history) Update throttle implementation.
+    // pub fn do_pending(&mut self) {
+    //     // TODO (tokio) This is ugly (depending on rendering cycles is unpredictable),
+    //     //  just making it work for now. History may need some scheduled events.
+    //     //  To do this asynchronously one would need to put id behind an Arc<RwLock<>> or
+    //     //  use Tokio here (and in the engine).
+    //     if let Some(throttle) = &self.throttle {
+    //         if !throttle.is_waiting(Instant::now()) {
+    //             let throttle = mem::replace(&mut self.throttle, None).unwrap();
+    //             self.update(throttle.changeset);
+    //             self.throttle = None;
+    //         }
+    //     }
+    // }
+
+    fn update(&mut self, applied_command: (EditCommandId, Vec<CommandDiff>)) {
+        let (command_id, diff) = applied_command;
+        if diff.is_empty() {
+            dbg!("No changes.");
             return;
         }
         // The changeset should be already applied to track by now.
-        self.push(changeset);
+        let log_entry = HistoryLogEntry {
+            base_version: self.version,
+            version: self.version + 1,
+            command_id,
+            diff,
+        };
+        self.push(log_entry);
+        // TODO also store a new snapshot here if necessary
+        //   (avoid long timeouts and long diff-only runs between snapshots).
         self.discard_tail(self.max_version);
     }
 
+    /// Save current version into history.
+    pub fn push(&mut self, log_entry: HistoryLogEntry) {
+        util::store(&log_entry, &self.diff_path(log_entry.version));
+        self.set_version(log_entry.version);
+        self.max_version = self.version;
+        self.write_meta();
+    }
+
     pub fn is_valid_version_id(v: VersionId) -> bool {
         v >= 0
     }
@@ -144,23 +164,23 @@ impl TrackHistory {
             }
             // Replays
             while self.version < version_id {
-                let diff: Patch = util::load(&self.diff_path(self.version + 1));
-                assert_eq!(diff.base_version, self.version);
-                assert!(diff.version > self.version);
-                let mut cs = Changeset::empty();
-                cs.add_all(&diff.changes);
-                track.patch(&cs);
-                self.set_version(diff.version);
+                let entry: HistoryLogEntry = util::load(&self.diff_path(self.version + 1));
+                assert_eq!(entry.base_version, self.version);
+                assert!(entry.version > self.version);
+                let mut changeset = Changeset::empty();
+                into_changeset(&track, &entry.diff, &mut changeset);
+                track.patch(&changeset);
+                self.set_version(entry.version);
             }
             // Rollbacks
             while self.version > version_id {
-                let diff: Patch = util::load(&self.diff_path(self.version));
-                assert_eq!(diff.version, self.version);
-                assert!(diff.base_version < self.version);
-                let mut cs = Changeset::empty();
-                cs.add_all(&diff.changes);
-                track.revert(&cs);
-                self.set_version(diff.base_version);
+                let entry: HistoryLogEntry = util::load(&self.diff_path(self.version));
+                assert_eq!(entry.version, self.version);
+                assert!(entry.base_version < self.version);
+                let mut changeset = Changeset::empty();
+                revert_diffs(&track, &entry.diff, &mut changeset);
+                track.patch(&changeset);
+                self.set_version(entry.base_version);
             }
         }
         dbg!(self.version, version_id);
@@ -168,21 +188,6 @@ impl TrackHistory {
         self.version == version_id
     }
 
-    /// Save current version into history.
-    pub fn push(&mut self, changeset: Changeset) {
-        let diff = Patch {
-            base_version: self.version,
-            version: self.version + 1,
-            changes: changeset.changes.values().cloned().collect(),
-        };
-        util::store(&diff, &self.diff_path(diff.version));
-        // TODO also store a new snapshot here if necessary
-        //   (avoid long timeouts and long diff-only runs between snapshots).
-        self.set_version(diff.version);
-        self.max_version = self.version;
-        self.write_meta();
-    }
-
     /// Maybe undo last edit action.
     pub fn undo(&mut self) {
         let prev_version_id = self.version - 1;
@@ -245,6 +250,7 @@ impl TrackHistory {
         dbg!("history directory", directory.to_string_lossy());
         Self {
             directory: directory.to_owned(),
+            id_seq: Arc::new(IdSeq::new(0)),
             version: 0,
             max_version: 0,
             track: Default::default(),
@@ -266,10 +272,18 @@ impl TrackHistory {
             );
         }
         let version = self.version;
-        self.update_track(None, |track, _changeset| {
-            track.import_smf(source_file);
-            util::store(&Snapshot::of_track(version, track), &starting_snapshot_path);
-        });
+
+        {
+            let id_seq = self.id_seq.clone();
+            self.update_track(|track| {
+                let mut patch = vec![];
+                for ev in import_smf(&id_seq, source_file) {
+                    patch.push(EventAction::Insert(ev));
+                }
+                util::store(&Snapshot::of_track(version, track), &starting_snapshot_path);
+                (EditCommandId::Load, vec![CommandDiff::Changeset { patch }])
+            });
+        }
         self.write_meta();
         self
     }
@@ -280,7 +294,7 @@ impl TrackHistory {
         let initial_version_id = 0;
         {
             let mut track = self.track.write().expect("Write to track.");
-            track.id_seq = IdSeq::new(meta.next_id);
+            self.id_seq = Arc::new(IdSeq::new(meta.next_id));
             track.reset(util::load(&self.snapshot_path(initial_version_id)));
         }
         self.set_version(initial_version_id);
@@ -298,7 +312,7 @@ impl TrackHistory {
 
     fn write_meta(&self) {
         let meta = Meta {
-            next_id: self.with_track(|t| t.id_seq.current()),
+            next_id: self.id_seq.current(),
             current_version: self.version,
             max_version: self.max_version,
         };
diff --git a/src/util.rs b/src/util.rs
index 9c75e9b..15c5ee1 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -8,15 +8,16 @@ use flate2::Compression;
 use serde::de::DeserializeOwned;
 use serde::Serialize;
 
+// Half-open range [a, b).
 pub type Range<T> = (T, T);
 
 // Could not find a simple library for this.
 #[allow(dead_code)]
-pub fn ranges_intersect<T: Ord>(a: Range<T>, b: Range<T>) -> bool {
+pub fn ranges_intersect<T: Ord>(a: &Range<T>, b: &Range<T>) -> bool {
     a.0 < b.1 && b.0 < a.1
 }
 
-pub fn range_contains<T: Ord>(r: Range<T>, x: T) -> bool {
+pub fn range_contains<T: Ord>(r: &Range<T>, x: T) -> bool {
     r.0 <= x && x < r.1
 }
 
@@ -37,7 +38,7 @@ impl IdSeq {
         IdSeq(atomic::AtomicU64::new(init))
     }
 
-    pub fn next(&mut self) -> u64 {
+    pub fn next(&self) -> u64 {
         self.0.fetch_add(1, atomic::Ordering::SeqCst)
     }
 
@@ -76,11 +77,11 @@ mod tests {
 
     #[test]
     fn check_range_contains() {
-        assert!(!range_contains((0, -1), 0));
-        assert!(!range_contains((0, 0), 0));
-        assert!(range_contains((0, 1), 0));
-        assert!(!range_contains((0, 1), 1));
-        assert!(!range_contains((0, 1), 2));
+        assert!(!range_contains(&(0, -1), 0));
+        assert!(!range_contains(&(0, 0), 0));
+        assert!(range_contains(&(0, 1), 0));
+        assert!(!range_contains(&(0, 1), 1));
+        assert!(!range_contains(&(0, 1), 2));
     }
 
     #[test]