diff --git a/deps/common/src/logseq/common/util.cljs b/deps/common/src/logseq/common/util.cljs index 4f4eb26c34..5deee4757b 100644 --- a/deps/common/src/logseq/common/util.cljs +++ b/deps/common/src/logseq/common/util.cljs @@ -298,3 +298,31 @@ (defn replace-first-ignore-case [s old-value new-value] (string/replace-first s (re-pattern (str "(?i)" (escape-regex-chars old-value))) new-value)) + + +(defn sort-coll-by-dependency + "Sort the elements in the collection based on dependencies. +coll: [{:id 1 :depend-on 2} {:id 2 :depend-on 3} {:id 3}] +get-elem-id-fn: :id +get-elem-dep-id-fn :depend-on +return: [{:id 3} {:id 2 :depend-on 3} {:id 1 :depend-on 2}]" + [get-elem-id-fn get-elem-dep-id-fn coll] + (let [id->elem (into {} (keep (juxt get-elem-id-fn identity)) coll) + id->dep-id (into {} (keep (juxt get-elem-id-fn get-elem-dep-id-fn)) coll) + all-ids (set (keys id->dep-id)) + sorted-ids + (loop [r [] + rest-ids all-ids + id (first rest-ids)] + (if-not id + r + (if-let [dep-id (id->dep-id id)] + ;; TODO: check no dep-cycle + (if-let [next-id (get rest-ids dep-id)] + (recur r rest-ids next-id) + (let [rest-ids* (disj rest-ids id)] + (recur (conj r id) rest-ids* (first rest-ids*)))) + ;; not found dep-id, so this id can be put into result now + (let [rest-ids* (disj rest-ids id)] + (recur (conj r id) rest-ids* (first rest-ids*))))))] + (mapv id->elem sorted-ids))) diff --git a/src/main/frontend/worker/undo_redo.cljs b/src/main/frontend/worker/undo_redo.cljs index 2b08063137..5a8c35ea22 100644 --- a/src/main/frontend/worker/undo_redo.cljs +++ b/src/main/frontend/worker/undo_redo.cljs @@ -7,6 +7,7 @@ [frontend.worker.db-listener :as db-listener] [frontend.worker.state :as worker-state] [logseq.common.config :as common-config] + [logseq.common.util :as common-util] [logseq.outliner.core :as outliner-core] [logseq.outliner.transaction :as outliner-tx] [malli.core :as m] @@ -27,9 +28,9 @@ so when undo, it will undo [ ] instead of [ ]") "boundary of one or more undo-ops. when one undo/redo will operate on all ops between two ::boundary") -(sr/defkeyword ::insert-block - "when a block is inserted, generate a ::insert-block undo-op. -when undo this op, the related block will be removed.") +(sr/defkeyword ::insert-blocks + "when some blocks are inserted, generate a ::insert-blocks undo-op. +when undo this op, the related blocks will be removed.") (sr/defkeyword ::move-block "when a block is moved, generate a ::move-block undo-op.") @@ -54,10 +55,10 @@ when undo this op, this original entity-map will be transacted back into db") [:multi {:dispatch first} [::boundary [:cat :keyword]] - [::insert-block + [::insert-blocks [:cat :keyword [:map - [:block-uuid :uuid]]]] + [:block-uuids [:sequential :uuid]]]]] [::move-block [:cat :keyword [:map @@ -113,25 +114,29 @@ when undo this op, this original entity-map will be transacted back into db") (seq (:block/tags m)) (update :block/tags (partial mapv :block/uuid))))) (defn- reverse-op + "return ops" [db op] (let [block-uuid (:block-uuid (second op))] (case (first op) - ::boundary op + ::boundary [op] - ::insert-block - [::remove-block - {:block-uuid block-uuid - :block-entity-map (->block-entity-map db [:block/uuid block-uuid])}] + ::insert-blocks + (mapv + (fn [block-uuid] + [::remove-block + {:block-uuid block-uuid + :block-entity-map (->block-entity-map db [:block/uuid block-uuid])}]) + (:block-uuids (second op))) ::move-block (let [b (d/entity db [:block/uuid block-uuid])] - [::move-block - {:block-uuid block-uuid - :block-origin-left (:block/uuid (:block/left b)) - :block-origin-parent (:block/uuid (:block/parent b))}]) + [[::move-block + {:block-uuid block-uuid + :block-origin-left (:block/uuid (:block/left b)) + :block-origin-parent (:block/uuid (:block/parent b))}]]) ::remove-block - [::insert-block {:block-uuid block-uuid}] + [[::insert-blocks {:block-uuids [block-uuid]}]] ::update-block (let [value-keys (set (keys (second op))) @@ -142,11 +147,11 @@ when undo this op, this original entity-map will be transacted back into db") (mapv :block/uuid (:block/tags block-entity))) block-origin-collapsed (when (contains? value-keys :block-origin-collapsed) (boolean (:block/collapsed? block-entity)))] - [::update-block - (cond-> {:block-uuid block-uuid} - (some? block-origin-content) (assoc :block-origin-content block-origin-content) - (some? block-origin-tags) (assoc :block-origin-tags block-origin-tags) - (some? block-origin-collapsed) (assoc :block-origin-collapsed block-origin-collapsed))])))) + [[::update-block + (cond-> {:block-uuid block-uuid} + (some? block-origin-content) (assoc :block-origin-content block-origin-content) + (some? block-origin-tags) (assoc :block-origin-tags block-origin-tags) + (some? block-origin-collapsed) (assoc :block-origin-collapsed block-origin-collapsed))]])))) (def ^:private apply-conj-vec (partial apply (fnil conj []))) @@ -248,27 +253,46 @@ when undo this op, this original entity-map will be transacted back into db") (assoc :block/updated-at (:block/updated-at block-entity-map)) (seq (:block/tags block-entity-map)) - (assoc :block/tags (mapv (partial vector :block/uuid) - (:block/tags block-entity-map))))] + (assoc :block/tags (some->> (:block/tags block-entity-map) + (map (partial vector :block/uuid)) + (d/pull-many @conn [:db/id]) + (keep :db/id))))] left-entity {:sibling? sibling? :keep-uuid? true})) (conj [:push-undo-redo]))))))) -(defmethod reverse-apply-op ::insert-block +(defn- sort-block-entities + "return nil when there are other children existing" + [block-entities] + (let [sorted-block-entities (common-util/sort-coll-by-dependency + :block/uuid (comp :block/uuid :block/parent) block-entities) + block-uuid-set (set (map :block/uuid sorted-block-entities))] + (when-not + (some ;; check no other children + (fn [ent] + (not-empty (set/difference (set (map :block/uuid (:block/_parent ent))) block-uuid-set))) + sorted-block-entities) + + sorted-block-entities))) + +(defmethod reverse-apply-op ::insert-blocks [op conn repo] - (let [[_ {:keys [block-uuid]}] op] - (when-let [block-entity (d/entity @conn [:block/uuid block-uuid])] - (when (empty? (:block/_parent block-entity)) ;if have children, skip - (some->> - (outliner-tx/transact! - {:gen-undo-op? false - :outliner-op :delete-blocks - :transact-opts {:repo repo - :conn conn}} - (outliner-core/delete-blocks! repo conn - (common-config/get-date-formatter (worker-state/get-config repo)) - [block-entity] - {:children? false})) - (conj [:push-undo-redo])))))) + (let [[_ {:keys [block-uuids]}] op] + (when-let [block-entities (->> block-uuids + (keep #(d/entity @conn [:block/uuid %])) + sort-block-entities + reverse + not-empty)] + (some->> + (outliner-tx/transact! + {:gen-undo-op? false + :outliner-op :delete-blocks + :transact-opts {:repo repo + :conn conn}} + (outliner-core/delete-blocks! repo conn + (common-config/get-date-formatter (worker-state/get-config repo)) + block-entities + {:children? false})) + (conj [:push-undo-redo]))))) (defmethod reverse-apply-op ::move-block [op conn repo] @@ -314,18 +338,35 @@ when undo this op, this original entity-map will be transacted back into db") (when r2 [:push-undo-redo r2])))))) +(defn- sort&merge-ops + [ops] + (let [groups (group-by first ops) + remove-ops (groups ::remove-block) + insert-ops (groups ::insert-blocks) + other-ops (apply concat (vals (dissoc groups ::remove-block ::insert-blocks))) + sorted-remove-ops (reverse + (common-util/sort-coll-by-dependency (comp :block-uuid second) + (comp :block/left :block-entity-map second) + remove-ops)) + insert-op (some->> (seq insert-ops) + (mapcat (fn [op] (:block-uuids (second op)))) + (hash-map :block-uuids) + (vector ::insert-blocks))] + (cond-> (concat sorted-remove-ops other-ops) + insert-op (conj insert-op)))) + (defn undo [repo page-block-uuid conn] (if-let [ops (not-empty (pop-undo-ops repo page-block-uuid))] (let [redo-ops-to-push (transient [])] (batch-tx/with-batch-tx-mode conn (doseq [op ops] - (let [rev-op (reverse-op @conn op) + (let [rev-ops (reverse-op @conn op) r (reverse-apply-op op conn repo)] (when (= :push-undo-redo (first r)) (some-> *undo-redo-info-for-test* (reset! {:op op :tx (second r)})) - (conj! redo-ops-to-push rev-op))))) - (when-let [rev-ops (not-empty (persistent! redo-ops-to-push))] + (apply conj! redo-ops-to-push rev-ops))))) + (when-let [rev-ops (not-empty (sort&merge-ops (persistent! redo-ops-to-push)))] (push-redo-ops repo page-block-uuid (cons boundary rev-ops))) nil) @@ -339,12 +380,12 @@ when undo this op, this original entity-map will be transacted back into db") (let [undo-ops-to-push (transient [])] (batch-tx/with-batch-tx-mode conn (doseq [op ops] - (let [rev-op (reverse-op @conn op) + (let [rev-ops (reverse-op @conn op) r (reverse-apply-op op conn repo)] (when (= :push-undo-redo (first r)) (some-> *undo-redo-info-for-test* (reset! {:op op :tx (second r)})) - (conj! undo-ops-to-push rev-op))))) - (when-let [rev-ops (not-empty (persistent! undo-ops-to-push))] + (apply conj! undo-ops-to-push rev-ops))))) + (when-let [rev-ops (not-empty (sort&merge-ops (persistent! undo-ops-to-push)))] (push-undo-ops repo page-block-uuid (cons boundary rev-ops))) nil) @@ -375,7 +416,7 @@ when undo this op, this original entity-map will be transacted back into db") (and add1? block-uuid (normal-block? entity-after)) - [[::insert-block {:block-uuid (:block/uuid entity-after)}]] + [[::insert-blocks {:block-uuids [(:block/uuid entity-after)]}]] (and (or add3? add4?) (normal-block? entity-after)) @@ -432,7 +473,8 @@ when undo this op, this original entity-map will be transacted back into db") (defn- generate-undo-ops [repo db-before db-after same-entity-datoms-coll id->attr->datom gen-boundary-op?] (when-let [page-block-uuid (find-page-block-uuid db-before db-after same-entity-datoms-coll)] - (let [ops (mapcat (partial entity-datoms=>ops db-before db-after id->attr->datom) same-entity-datoms-coll)] + (let [ops (mapcat (partial entity-datoms=>ops db-before db-after id->attr->datom) same-entity-datoms-coll) + ops (sort&merge-ops ops)] (when (seq ops) (push-undo-ops repo page-block-uuid (if gen-boundary-op? (cons boundary ops) ops)))))) @@ -468,4 +510,5 @@ when undo this op, this original entity-map will be transacted back into db") :n n}))) (remove-watch (:undo/repo->pege-block-uuid->undo-ops @worker-state/*state) :xxx) - (remove-watch (:undo/repo->pege-block-uuid->redo-ops @worker-state/*state) :xxx)) + (remove-watch (:undo/repo->pege-block-uuid->redo-ops @worker-state/*state) :xxx) + ) diff --git a/src/test/frontend/worker/undo_redo_test.cljs b/src/test/frontend/worker/undo_redo_test.cljs index 072164849d..76f9a97cc5 100644 --- a/src/test/frontend/worker/undo_redo_test.cljs +++ b/src/test/frontend/worker/undo_redo_test.cljs @@ -42,8 +42,8 @@ (defn- gen-insert-block-op [db] (gen/let [block-uuid (gen-block-uuid db)] - [:frontend.worker.undo-redo/insert-block - {:block-uuid block-uuid}])) + [:frontend.worker.undo-redo/insert-blocks + {:block-uuids [block-uuid]}])) (defn- gen-remove-block-op [db] @@ -112,9 +112,11 @@ (assert (some? (d/entity current-db [:block/uuid (:block-uuid (second op))])) {:op op :tx-data (:tx-data tx)}) - :frontend.worker.undo-redo/insert-block - (assert (nil? (d/entity current-db [:block/uuid (:block-uuid (second op))])) - {:op op :tx-data (:tx-data tx) :x (keys tx)}) + :frontend.worker.undo-redo/insert-blocks + (let [entities (map #(d/entity current-db [:block/uuid %]) (:block-uuids (second op)))] + (assert (every? nil? entities) + {:op op :tx-data (:tx-data tx) :x (keys tx)})) + :frontend.worker.undo-redo/remove-block (assert (some? (d/entity current-db [:block/uuid (:block-uuid (second op))])) {:op op :tx-data (:tx-data tx) :x (keys tx)}) @@ -174,3 +176,16 @@ (is (= origin-graph-block-set (get-db-block-set @conn))))) )) + +;;; TODO: generate outliner-ops then undo/redo/validate +;; (deftest undo-redo-single-step-check-gen-test +;; (let [conn (db/get-db false) +;; all-remove-ops (gen/generate (gen/vector (gen-op @conn {:remove-block-op 1000}) 20))] +;; (#'undo-redo/push-undo-ops test-helper/test-db-name-db-version page-uuid all-remove-ops) +;; (loop [] +;; (when (not= :frontend.worker.undo-redo/empty-undo-stack +;; (undo-redo/undo test-helper/test-db-name-db-version page-uuid conn)) +;; (recur))) +;; (prn :init-blocks (d/entity @conn )) + +;; ))