From d142cf227eaf46107bf8d204242ff3cc24df1650 Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Tue, 14 May 2024 17:57:31 +0800 Subject: [PATCH] refactor: new undo/redo implementation Previous undo/redo requires to handle every editor operation including any new ops. The new implemenation focus on db attributes instead of ops, it assumes that most attributes can be safely overwritten except :block/parent, block/order and other ref-type attributes. It works likes: 1. If a block has been moved by another client (:block/parent or :block/order has been changed), the whole tx will be skipped 2. If a refed entity has been deleted, the datom will be skipped --- src/main/frontend/db_worker.cljs | 15 +- .../frontend/modules/outliner/pipeline.cljs | 4 - src/main/frontend/worker/db_listener.cljs | 2 +- src/main/frontend/worker/state.cljs | 7 +- src/main/frontend/worker/undo_redo.cljs | 15 +- src/main/frontend/worker/undo_redo2.cljs | 202 ++++++++++++++++++ 6 files changed, 225 insertions(+), 20 deletions(-) create mode 100644 src/main/frontend/worker/undo_redo2.cljs diff --git a/src/main/frontend/db_worker.cljs b/src/main/frontend/db_worker.cljs index 003e7c8c4c..aad555beb6 100644 --- a/src/main/frontend/db_worker.cljs +++ b/src/main/frontend/db_worker.cljs @@ -20,7 +20,8 @@ [frontend.worker.rtc.op-mem-layer :as op-mem-layer] [frontend.worker.search :as search] [frontend.worker.state :as worker-state] - [frontend.worker.undo-redo :as undo-redo] + ;; [frontend.worker.undo-redo :as undo-redo] + [frontend.worker.undo-redo2 :as undo-redo] [frontend.worker.util :as worker-util] [logseq.db :as ldb] [logseq.db.sqlite.common-db :as sqlite-common-db] @@ -657,18 +658,18 @@ (js/Promise. (rtc-core2/new-task--snapshot-list token graph-uuid)))) (undo - [_this repo page-block-uuid-str] + [_this repo _page-block-uuid-str] (when-let [conn (worker-state/get-datascript-conn repo)] - (ldb/write-transit-str (undo-redo/undo repo (uuid page-block-uuid-str) conn)))) + (ldb/write-transit-str (undo-redo/undo repo conn)))) (redo - [_this repo page-block-uuid-str] + [_this repo _page-block-uuid-str] (when-let [conn (worker-state/get-datascript-conn repo)] - (ldb/write-transit-str (undo-redo/redo repo (uuid page-block-uuid-str) conn)))) + (ldb/write-transit-str (undo-redo/redo repo conn)))) (record-editor-info - [_this repo page-block-uuid-str editor-info-str] - (undo-redo/record-editor-info! repo (uuid page-block-uuid-str) (ldb/read-transit-str editor-info-str)) + [_this repo _page-block-uuid-str editor-info-str] + (undo-redo/record-editor-info! repo (ldb/read-transit-str editor-info-str)) nil) (keep-alive diff --git a/src/main/frontend/modules/outliner/pipeline.cljs b/src/main/frontend/modules/outliner/pipeline.cljs index 4328a3cddb..f9ac7caa59 100644 --- a/src/main/frontend/modules/outliner/pipeline.cljs +++ b/src/main/frontend/modules/outliner/pipeline.cljs @@ -52,10 +52,6 @@ (state/set-state! :editor/next-edit-block nil))) (react/refresh! repo affected-keys) - ;; (when-let [state (:ui/restore-cursor-state @state/state)] - ;; (when (or undo? redo?) - ;; (restore-cursor-and-app-state! state undo?) - ;; (state/set-state! :ui/restore-cursor-state nil))) (state/set-state! :editor/start-pos nil) diff --git a/src/main/frontend/worker/db_listener.cljs b/src/main/frontend/worker/db_listener.cljs index f2bb9c22c0..e07e6e3731 100644 --- a/src/main/frontend/worker/db_listener.cljs +++ b/src/main/frontend/worker/db_listener.cljs @@ -85,7 +85,6 @@ generate undo ops.") (let [handlers (if (seq handler-keys) (select-keys (methods listen-db-changes) handler-keys) (methods listen-db-changes))] - (prn :listen-db-changes! (keys handlers)) (d/unlisten! conn ::listen-db-changes!) (d/listen! conn ::listen-db-changes! (fn [{:keys [tx-data _db-before _db-after tx-meta] :as tx-report}] @@ -106,6 +105,7 @@ generate undo ops.") :tx-meta tx-meta :tx-data tx-data :db-before db-before) + ;; TODO: move to RTC because other modules do not need these datom-vec-coll (map vec tx-data) id->same-entity-datoms (group-by first datom-vec-coll) id-order (distinct (map first datom-vec-coll)) diff --git a/src/main/frontend/worker/state.cljs b/src/main/frontend/worker/state.cljs index a628cf94e6..3fce68f808 100644 --- a/src/main/frontend/worker/state.cljs +++ b/src/main/frontend/worker/state.cljs @@ -26,7 +26,12 @@ :rtc/downloading-graph? false :undo/repo->page-block-uuid->undo-ops (atom {}) - :undo/repo->page-block-uuid->redo-ops (atom {})})) + :undo/repo->page-block-uuid->redo-ops (atom {}) + + ;; new implementation + :undo/repo->ops (atom {}) + :redo/repo->ops (atom {}) + })) (defonce *rtc-ws-url (atom nil)) diff --git a/src/main/frontend/worker/undo_redo.cljs b/src/main/frontend/worker/undo_redo.cljs index b2cc952a91..64e6841e43 100644 --- a/src/main/frontend/worker/undo_redo.cljs +++ b/src/main/frontend/worker/undo_redo.cljs @@ -553,13 +553,14 @@ when undo this op, this original entity-map will be transacted back into db") (:gen-undo-boundary-op? tx-meta true) tx-meta))) -(defn record-editor-info! - [repo page-block-uuid editor-info] - (swap! (:undo/repo->page-block-uuid->undo-ops @worker-state/*state) - update-in [repo page-block-uuid] - (fn [stack] - (when (seq stack) - (conj (vec stack) [::record-editor-info editor-info]))))) +(comment + (defn record-editor-info! + [repo page-block-uuid editor-info] + (swap! (:undo/repo->page-block-uuid->undo-ops @worker-state/*state) + update-in [repo page-block-uuid] + (fn [stack] + (when (seq stack) + (conj (vec stack) [::record-editor-info editor-info])))))) ;;; listen db changes and push undo-ops (ends) diff --git a/src/main/frontend/worker/undo_redo2.cljs b/src/main/frontend/worker/undo_redo2.cljs new file mode 100644 index 0000000000..9359ee2c8b --- /dev/null +++ b/src/main/frontend/worker/undo_redo2.cljs @@ -0,0 +1,202 @@ +(ns frontend.worker.undo-redo2 + "Undo redo new implementation" + (:require [datascript.core :as d] + [frontend.worker.db-listener :as db-listener] + [frontend.worker.state :as worker-state])) + +(def ^:private boundary [::boundary]) + +(defonce max-stack-length 500) +(defonce *undo-ops (:undo/repo->ops @worker-state/*state)) +(defonce *redo-ops (:redo/repo->ops @worker-state/*state)) + +(defn- conj-ops + [col ops] + (let [result (apply (fnil conj []) col ops)] + (if (>= (count result) max-stack-length) + (subvec result 0 (/ max-stack-length 2)) + result))) + +(defn- pop-ops-helper + [stack] + (loop [ops [] + stack stack] + (let [last-op (peek stack)] + (cond + (empty? stack) + nil + (= boundary last-op) + [(reverse (conj ops last-op)) (pop stack)] + :else + (recur (conj ops last-op) (pop stack)))))) + +(defn- push-undo-ops + [repo ops] + (swap! *undo-ops update repo conj-ops ops)) + +(defn- pop-undo-ops + [repo] + (let [undo-stack (get @*undo-ops repo) + [ops undo-stack*] (pop-ops-helper undo-stack)] + (swap! *undo-ops assoc repo undo-stack*) + ops)) + +(defn- empty-undo-stack? + [repo] + (empty? (get @*undo-ops repo))) + +(defn- empty-redo-stack? + [repo] + (empty? (get @*redo-ops repo))) + +(defn- push-redo-ops + [repo ops] + (swap! *redo-ops update repo conj-ops ops)) + +(defn- pop-redo-ops + [repo] + (let [undo-stack (get @*redo-ops repo) + [ops redo-stack*] (pop-ops-helper undo-stack)] + (swap! *redo-ops assoc repo redo-stack*) + ops)) + +(defn get-reversed-datoms + [conn redo? {:keys [tx-data tx-meta added-ids retracted-ids]}] + (try + (let [e->datoms (->> (if redo? tx-data (reverse tx-data)) + (group-by :e)) + schema (:schema @conn)] + (->> + (mapcat + (fn [[e datoms]] + (cond + ;; block has been moved by another client + (and + (= :move-blocks (:outliner-op tx-meta)) + (let [b (d/entity @conn e) + cur-parent (:db/id (:block/parent b)) + cur-order (:block/order b) + move-datoms (filter (fn [d] (contains? #{:block/parent :block/order} (:a d))) datoms) + cur [cur-parent cur-order]] + (when (and cur-parent cur-order) + (let [before-parent (some (fn [d] (when (and (= :block/parent (:a d)) (not (:added d))) (:v d))) move-datoms) + after-parent (some (fn [d] (when (and (= :block/parent (:a d)) (:added d)) (:v d))) move-datoms) + before-order (some (fn [d] (when (and (= :block/order (:a d)) (not (:added d))) (:v d))) move-datoms) + after-order (some (fn [d] (when (and (= :block/order (:a d)) (:added d)) (:v d))) move-datoms) + before [before-parent before-order] + after [after-parent after-order]] + (if redo? + (not= cur before) + (not= cur after)))))) + ;; skip this tx + (throw (ex-info "This block has been moved" + {:error :block-moved})) + + ;; The entity should be deleted instead of retracting its attributes + (or (and (contains? retracted-ids e) redo?) + (and (contains? added-ids e) (not redo?))) + [[:db/retractEntity e]] + + :else + (keep + (fn [[id attr value _tx add?]] + (let [ref? (= :db.type/ref (get-in schema [attr :db/valueType])) + op (if (or (and redo? add?) (and (not redo?) (not add?))) + :db/add + :db/retract)] + (when-not (and ref? + (not (d/entity @conn value)) + (not (and (retracted-ids value) (not redo?))) + (not (and (added-ids value) redo?))) ; ref has been deleted + [op id attr value]))) + datoms))) + e->datoms) + (remove nil?))) + (catch :default e + (when (not= :block-moved (:error (ex-data e))) + (throw e))))) + +(defn undo + [repo conn] + (if-let [ops (not-empty (pop-undo-ops repo))] + (let [{:keys [tx-data tx-meta] :as data} (some #(when (= ::db-transact (first %)) + (second %)) ops)] + (when (seq tx-data) + (let [reversed-tx-data (get-reversed-datoms conn false data) + tx-meta' (-> tx-meta + (dissoc :pipeline-replace? + :batch-tx/batch-tx-mode?) + (assoc + :gen-undo-ops? false + :undo? true))] + (when (seq reversed-tx-data) + (d/transact! conn reversed-tx-data tx-meta') + (push-redo-ops repo ops) + (let [editor-cursors (->> (filter #(= ::record-editor-info (first %)) ops) + (map second)) + block-content (:block/content (d/entity @conn [:block/uuid (:block-uuid (first editor-cursors))]))] + {:undo? true + :editor-cursors editor-cursors + :block-content block-content}))))) + + (when (empty-undo-stack? repo) + (prn "No further undo information") + ::empty-undo-stack))) + +(defn redo + [repo conn] + (if-let [ops (not-empty (pop-redo-ops repo))] + (let [{:keys [tx-meta tx-data] :as data} (some #(when (= ::db-transact (first %)) + (second %)) ops)] + (when (seq tx-data) + (let [reversed-tx-data (get-reversed-datoms conn true data) + tx-meta' (-> tx-meta + (dissoc :pipeline-replace? + :batch-tx/batch-tx-mode?) + (assoc + :gen-undo-ops? false + :redo? true))] + (d/transact! conn reversed-tx-data tx-meta') + (push-undo-ops repo ops) + (let [editor-cursors (->> (filter #(= ::record-editor-info (first %)) ops) + (map second)) + block-content (:block/content (d/entity @conn [:block/uuid (:block-uuid (last editor-cursors))]))] + {:redo? true + :editor-cursors editor-cursors + :block-content block-content})))) + + (when (empty-redo-stack? repo) + (prn "No further redo information") + ::empty-redo-stack))) + +(defn record-editor-info! + [repo editor-info] + (swap! *undo-ops + update repo + (fn [stack] + (when (seq stack) + (conj stack [::record-editor-info editor-info]))))) + +(defmethod db-listener/listen-db-changes :gen-undo-ops + [_ {:keys [repo tx-data tx-meta db-after db-before]}] + (let [{:keys [outliner-op]} tx-meta] + (when (and outliner-op (not (false? (:gen-undo-ops? tx-meta)))) + (let [editor-info (:editor-info tx-meta) + all-ids (distinct (map :e tx-data)) + retracted-ids (set + (filter + (fn [id] (and (nil? (d/entity db-after id)) (d/entity db-before id))) + all-ids)) + added-ids (set + (filter + (fn [id] (and (nil? (d/entity db-before id)) (d/entity db-after id))) + all-ids)) + ops (->> [boundary + (when editor-info [::record-editor-info editor-info]) + [::db-transact + {:tx-data tx-data + :tx-meta tx-meta + :added-ids added-ids + :retracted-ids retracted-ids}]] + (remove nil?))] + (push-undo-ops repo ops)))))