github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

📚 Call the ids a comparison runs in an id space, not a frame

Browse Source 
The branch comments called the set of ids a document's references are
written in (main's file id and media rows, or the branch's) a frame.
Penpot's code uses frame for a board, through `:frame-id`, so a reader
could not tell which sense a comment meant. The comments, the
docstrings of `get-base-data`, `branch-base-data`, and `remap-changes`,
and one test name now say id space. Every board-sense use of frame is
unchanged.

AI-assisted-by: mixed models
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>
This commit is contained in:
Álvaro Tejero Cantero 2026-10-03 23:19:44 +02:00
parent 325f963685
commit fe64834d1d
No known key found for this signature in database
6 changed files with 38 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
backend/src/app/rpc/commands/files_branch.clj
View File

@ -270,11 +270,11 @@
the op log over it, so this function would decode the same snapshot a
second time.
The snapshot stores the branch's frame — its media references name the
branch's own paired rows, what the replay reproduces — while a
comparison runs in main's frame, so the media ids are normalized back
here through the pairs, the way `branch-id-map` normalizes the branch's
own data."
The snapshot stores the branch's id space — its media references name
the branch's own paired rows, what the replay reproduces — while a
comparison runs in main's id space, so the media ids are normalized
back here through the pairs, the way `branch-id-map` normalizes the
branch's own data."
[cfg {:keys [source-file-id branch-file-id base-snapshot-id] :as branch}]
(when-not base-snapshot-id
(ex/raise :type :not-found
@ -297,7 +297,7 @@
nil.
Like `get-base-data`, it comes back with the media ids normalized into
main's frame: the snapshot names the branch's own paired rows, the
main's id space: the snapshot names the branch's own paired rows, the
comparison runs against main's."
[cfg source-file-id {:keys [id] :as branch-file}]
(-> (or (::bfc/base-data branch-file)
@ -1283,14 +1283,14 @@
;; Two id maps, one per direction of the crossing.
;;
;; The COMPARISON runs in main's frame, the frame the merge
;; base was captured in, the frame the compare view uses
;; (`files_branch.clj::branch-id-map` in
;; `files_branch.clj::get-branch-diff`), and the only frame
;; The COMPARISON runs in main's id space, the id space the
;; merge base was captured in, the id space the compare view
;; uses (`files_branch.clj::branch-id-map` in
;; `files_branch.clj::get-branch-diff`), and the only id space
;; the branch's inherited content agrees with. A branch stores
;; no data, so `binfile.clj::branch-file-data` replays its op
;; log over that base and everything it inherited carries the
;; base's ids. Comparing in the branch's frame instead
;; base's ids. Comparing in the branch's id space instead
;; rewrites main's and the base's local references to the
;; branch file id, and a self reference in main is then
;; unequal to the branch's own copy of it, so every shape
@ -1301,8 +1301,8 @@
branch-cmp (bm/remap-refs (:data branch-file)
(branch-id-map cfg branch-file-id main-id))
;; The WRITE runs in the branch's frame: a reference is only a
;; reference in the file it names, so main's file id and
;; The WRITE runs in the branch's id space: a reference is only
;; a reference in the file it names, so main's file id and
;; main's media rows are re-pointed at the branch's ids before
;; the change reaches the branch. Media ADDED on main has no
;; branch row yet, so it gets a fresh id here and its row is
@ -1310,7 +1310,7 @@
pairs (media-pairs cfg main-id branch-file-id)
new-media (unpaired-media-rows cfg main-id main-raw pairs)
fresh-map (into {} (map (fn [row] [(:id row) (uuid/next)])) new-media)
;; the branch's OWN frame: every media row the branch
;; the branch's OWN id space: every media row the branch
;; references is one of its own — a paired copy or one of
;; the fresh copies made below. The write and the pinned
;; base name those; `apply-map` adds the file id on top for
@ -1327,13 +1327,13 @@
reposition-base!
(fn [ts branch-revn base-data]
;; the base is pinned in the branch's frame — its media
;; references name the branch's own rows — because it is
;; what the op log replays over: the derived document
;; carries that frame, so a base naming main's rows would
;; make every paired media read as deleted+added on the
;; next squash. The comparisons normalize it back to
;; main's frame on read (`get-base-data`/`branch-base-data`).
;; the base is pinned in the branch's id space — its media
;; references name the branch's own rows — because it is what
;; the op log replays over: the derived document carries that
;; id space, so a base naming main's rows would make every
;; paired media read as deleted+added on the next squash. The
;; comparisons normalize it back to main's id space on read
;; (`get-base-data`/`branch-base-data`).
(let [new-base (fsnap/create! cfg (assoc main-file :data base-data)
{:label (str "branch-base/" (:name branch))
:created-by "system"
@ -1415,8 +1415,8 @@
;; which re-derives the net against the new base.
(db/delete! conn :file-branch-change {:branch-id branch-id})
;; in sync, so no row is copied and the pairs are all
;; the frame the base needs: unpaired rows stay named
;; by the id main owns, which is what the branch
;; the id space the base needs: unpaired rows stay
;; named by the id main owns, which is what the branch
;; references while it holds no copy
(reposition-base! ts (:revn branch-file) (bm/remap-refs main-raw pairs))
(audited {:status :updated :revn (:revn branch-file)} tpoint :update-from-main))
@ -1439,8 +1439,8 @@
(copy-media-rows! conn branch-file-id fresh-map new-media)
(let [applied (bm/remap-changes changes apply-map)
;; the write lands in the branch's frame, so the
;; document is moved there first — its paired
;; the write lands in the branch's id space, so
;; the document is moved there first — its paired
;; media refs re-pointed at the branch's own
;; rows — or the remapped changes would name ids
;; it does not hold and apply to nothing (a

10
backend/test/backend_tests/rpc_file_branch_oplog_test.clj
View File

@ -210,13 +210,13 @@
(ct/is-after? deleted-at (ct/in-future {:days 400}))))
rows)))))))
(t/deftest update-from-main-compares-in-main-frame
(t/deftest update-from-main-compares-in-main-id-space
;; A branch stores no data: its document is the base snapshot plus the op
;; log, so every reference it inherited names MAIN. A local reference is a
;; self reference in both files, because a branch keeps main's colours and
;; typographies under the same entity ids, so the comparison must run in
;; main's frame and the change that lands in the branch must still name the
;; branch.
;; main's id space and the change that lands in the branch must still name
;; the branch.
(with-redefs [cf/flags (conj cf/flags :branching)]
(let [profile (th/create-profile* 1 {:is-active true})
proj-id (:default-project-id profile)
@ -252,7 +252,7 @@
:parent-id uuid/zero :frame-id uuid/zero})
(assoc :fills (fills main-id "#ff0000")))})
(let [create (create-branch* profile main-id "main-frame")
(let [create (create-branch* profile main-id "main-id-space")
branch-id (:id create)
branch-file-id (:branch-file-id create)]
@ -273,7 +273,7 @@
(t/is (= "#00ff00" (get-in out [:result :data :pages-index page-id
:objects shape-id :fills 0 :fill-color]))))
;; main's own change arrives through the repositioned base, so the
;; net the update writes is empty. A ref left in main's frame would
;; net the update writes is empty. A ref left in main's id space would
;; make this shape differ from the new base and put it in the log.
(t/is (empty? (oplog-rows branch-file-id))))

4
backend/test/backend_tests/rpc_file_branch_test.clj
View File

@ -1141,7 +1141,7 @@
(t/is (some? img-id))
;; the colour's image keeps resolving to the right content (the
;; document names the branch's own paired copy: the base is pinned
;; in the branch's frame and the op log replays over it)
;; in the branch's id space and the op log replays over it)
(let [row (th/db-get :file-media-object {:id img-id})]
(t/is (= (:id sobj) (:media-id row)))))
@ -1154,7 +1154,7 @@
(t/testing "the update logs no phantom branch change"
;; the colour image id must be re-pointed at the copy like every
;; other ref (`bm/remap-changes`); left in main's frame it makes
;; other ref (`bm/remap-changes`); left in main's id space it makes
;; the colour differ from the new base and lands in the op log
(t/is (empty? (th/db-query :file-branch-change {:branch-id branch-id})))))))

2
common/src/app/common/files/branch_merge.cljc
View File

@ -1099,7 +1099,7 @@
(defn remap-changes
"Rewrite the cross-file references of a change vector through `id-map`,
the counterpart of `remap-refs` for the changes `compute-changes` emits.
A change computed in the source file's frame has to be applied in the
A change computed in the source file's id space has to be applied in the
target's, because a reference is only a reference in the file it names: a
fill whose `:fill-color-ref-file` names another file loses its link, and so
does an instance whose `:component-file` does.

4
frontend/src/app/main/data/workspace/branches.cljs
View File

@ -439,7 +439,7 @@
Those conflicts are authoritative for the whole modal: they are the very
ones the command's own `resolved?` matches against, while the diff the modal
can fetch on its own is computed in another id frame and may list a
can fetch on its own is computed in another id space and may list a
different set, or none at all. Keeping them in the store also lets the
bulk actions resolve the same set the modal shows."
[{:keys [branch mode conflicts]}]
@ -491,7 +491,7 @@
;; the returned conflicts are authoritative: they are the very
;; ones the command computes `resolved?` against, and the diff
;; the modal could fetch on its own is computed in another id
;; frame, which can disagree with them (vcs:tp-update-conflict-frame)
;; space, which can disagree with them (vcs:tp-update-conflict-frame)
:conflicts (rx/of (open-conflict-resolutions {:branch branch
:mode :update
:conflicts conflicts}))

6
frontend/src/app/main/ui/workspace/sidebar/branches.cljs
View File

@ -1600,9 +1600,9 @@
(if ago (str ago " · " you) you))
;; the command that opened this modal returned its own conflict set,
;; computed in the frame it resolves against; the diff this dialog can
;; fetch may have been computed in another frame and disagree, which
;; left the dialog showing 0 of 0 with an Apply that could never
;; computed in the id space it resolves against; the diff this dialog
;; can fetch may have been computed in another id space and disagree,
;; which left the dialog showing 0 of 0 with an Apply that could never
;; enable (vcs:tp-update-conflict-frame). Prefer what the command said.
conflicts (if (seq conflicts) conflicts (:conflicts diff))