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

🐛 Merge the tokens source of a branch

Browse Source 
The merge engine never compared the file's tokens source, so a branch
that switched its tokens to a library, or back to its own, lost that
change on merge, and main's change was lost on an update from main.

The source is now a mergeable kind, :tokens-source, compared raw (nil
means the file itself, and a value naming the file reads as nil). A
change on one side applies as a :set-tokens-source change, and a
change on both sides is a conflict resolved like the base font size.

remap-refs now also rewrites the document's own :id and its
:tokens-source through the id map, and remap-changes rewrites the
:library-id of a :set-tokens-source change, so a branch document moved
into main's id space names main as itself. The compare panel labels
the new kind.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Álvaro Tejero Cantero 2026-10-28 12:31:05 +01:00
parent 90525f91a8
commit 7f07f13940
No known key found for this signature in database
4 changed files with 182 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -428,6 +428,24 @@
(case option
:base-font-size {:type :set-base-font-size :base-font-size value}))
(defn- tokens-source
"The file's tokens source as the merge compares it, keyed by
`:tokens-source`: the RAW `:tokens-source` value, nil meaning the file
itself. A value naming one of `self-ids` (the `:id`s of the compared
documents, all ids of the same logical file) also reads as nil, so a
side that names itself explicitly does not differ from one that leaves
the attribute out. The effective source is not compared: it falls back
to the document's own `:id`, and that differs between a branch and its
source."
[data self-ids]
(let [source (:tokens-source data)]
{:tokens-source (when-not (contains? self-ids source) source)}))
(defn- self-ids
"The data `:id`s of the compared documents."
[& datas]
(into #{} (keep :id) datas))
;; --- Shape tree ---
;;
;; `three-way-entities` classifies shapes one by one, while containment
@ -1073,7 +1091,11 @@
`:typography-ref-file`, shadow/grid `:file-id`) plus the media refs
(shape `:metadata`/`:fill-image`/`:stroke-image` ids, the `:media`
collection keys and library-color `:image` ids). It is the same surface
`binfile.common/process-file` remaps when it copies a file.
`binfile.common/process-file` remaps when it copies a file. The
document's own `:id` and its `:tokens-source` are rewritten too, so a
document moved into another file's id space names that file as itself,
and a tokens source that named the old file names the new one, which
`set-tokens-source` and `get-effective-tokens-source` read as self.
A branch stores no data of its own: its state is its source's base
snapshot with the branch's op log replayed over it
@ -1091,6 +1113,8 @@
data
(let [lookup #(get id-map % %)]
(-> data
(d/update-when :id #(if (uuid? %) (lookup %) %))
(d/update-when :tokens-source lookup)
(d/update-when :pages-index #(cfh/relink-refs % lookup))
(d/update-when :components #(cfh/relink-refs % lookup))
(d/update-when :media
@ -1149,12 +1173,14 @@
complete on their own. The payloads it cannot see are the values that ARE
the reference: a media object's `:id` (its `:media-id` is a storage key and
must not be touched), a library color's `:image` id, a `:del-media`'s
top-level `:id`, and an operation value that is the reference itself. The
top-level `:id` of the other delete changes (`:del-color`,
`:del-typography`, `:del-component`, `:del-obj`, `:del-page`) is NOT such a
value: those name entities the two files share by id, never a media row,
so they are left alone — only a media id or the file id is ever a key of
`id-map`. Token changes are left alone: no id in `id-map` is a token id."
top-level `:id`, a `:set-tokens-source`'s `:library-id`, and an operation
value that is the reference itself. A `:set-tokens-source`'s `:file-id` is
a `:file-id` key, which `relink-refs` rewrites. The top-level `:id` of the
other delete changes (`:del-color`, `:del-typography`, `:del-component`,
`:del-obj`, `:del-page`) is NOT such a value: those name entities the two
files share by id, never a media row, so they are left alone — only a
media id or the file id is ever a key of `id-map`. The other token changes
are left alone: no id in `id-map` is a token id."
[changes id-map]
(if (empty? id-map)
changes
@ -1168,6 +1194,10 @@
(uuid? (:id (:object change)))
(update-in [:object :id] lookup)
(and (= :set-tokens-source (:type change))
(uuid? (:library-id change)))
(update :library-id lookup)
(uuid? (get-in change [:color :image :id]))
(update-in [:color :image :id] lookup)
@ -1294,6 +1324,11 @@
{:kind :file-plugin})
(three-way-entities (file-options base) (file-options theirs) (file-options ours)
{:kind :file-option})
;; the tokens source: a kind of its own, compared raw
(let [ids (self-ids base theirs ours)]
(three-way-entities (tokens-source base ids) (tokens-source theirs ids)
(tokens-source ours ids)
{:kind :tokens-source}))
(diff-pages base theirs ours only-pages dir)
(diff-tokens base theirs ours)]
{:keys [changes conflicts]} (merge-results results)
@ -1345,7 +1380,8 @@
there would hide them.
The `:tokens-lib` is diffed as well (`diff-tokens`), so token changes
are reported here with their own kinds."
are reported here with their own kinds. The file's tokens source is
reported under kind `:tokens-source`, entity id `:tokens-source`."
([base main branch dir] (compute-merge* base main branch dir nil))
([base main branch dir {:keys [only-pages]}]
(compute-merge* base main branch dir only-pages)))
@ -1373,7 +1409,7 @@
#{:color :typography :media :shape :token :token-set :token-set-rename :token-set-order
:token-theme :token-active-themes :token-active-sets
:page :page-order :page-guide :page-flow :page-grid :page-plugin :component
:file-plugin :file-option})
:file-plugin :file-option :tokens-source})
(defn unsupported-kinds
"Set of change kinds present in `changes` that `compute-changes`
@ -1999,6 +2035,16 @@
(file-options base) (file-options main) (file-options branch)
set-option-change set-option-change (fn [_] nil))
;; tokens source: always present on every side (nil = self), so
;; never added or deleted. The change names the target document.
ids (self-ids base main branch)
set-source (fn [_ library-id]
{:type :set-tokens-source :file-id (:id main) :library-id library-id})
source-changes (flat :tokens-source {}
(tokens-source base ids) (tokens-source main ids)
(tokens-source branch ids)
set-source set-source (fn [_] nil))
;; pages: add (full page incl. objects) / delete + rename (mod-page)
bpi (:pages-index base) mpi (:pages-index main) opi (:pages-index branch)
bpids (set (keys bpi)) mpids (set (keys mpi)) opids (set (keys opi))
@ -2257,6 +2303,7 @@
:changes (vec (concat page-presence components shapes page-meta-changes
page-guides page-flows page-grids page-plugins page-order-changes
colors typos media file-plugins (filterv some? file-opts)
(filterv some? source-changes)
set-presence set-rename-changes set-order-changes
token-vals themes
active-changes))})))

117
common/test/common_tests/files_branch_merge_test.cljc
View File

@ -324,6 +324,34 @@
(t/is (= {media-new {:id media-new :name "img"}} (:media data')))
(t/is (= media-new (get-in data' [:colors :c1 :image :id])))))
(t/deftest remap-refs-remaps-document-id-and-tokens-source
(let [branch-id (uuid/next)
main-id (uuid/next)
ext-lib (uuid/next)
media-old (uuid/next)]
(t/testing "the file id map moves the document and its self-named source"
(let [data' (bm/remap-refs (assoc (mkdata {}) :id branch-id :tokens-source branch-id)
{branch-id main-id})]
(t/is (= main-id (:id data')))
(t/is (= main-id (:tokens-source data')))
(t/is (cfo/effective-tokens-source? data' main-id))))
(t/testing "a library tokens source outside the map stays"
(let [data' (bm/remap-refs (assoc (mkdata {}) :id branch-id :tokens-source ext-lib)
{branch-id main-id})]
(t/is (= main-id (:id data')))
(t/is (= ext-lib (:tokens-source data')))))
(t/testing "a media-only map leaves the document id alone"
(let [data' (bm/remap-refs (assoc (mkdata {}) :id branch-id) {media-old (uuid/next)})]
(t/is (= branch-id (:id data')))
(t/is (not (contains? data' :tokens-source)))))
(t/testing "remap-changes maps a set-tokens-source change"
(t/is (= [{:type :set-tokens-source :file-id branch-id :library-id branch-id}]
(bm/remap-changes [{:type :set-tokens-source :file-id main-id :library-id main-id}]
{main-id branch-id})))
(t/is (= [{:type :set-tokens-source :file-id branch-id :library-id nil}]
(bm/remap-changes [{:type :set-tokens-source :file-id main-id :library-id nil}]
{main-id branch-id}))))))
(t/deftest merge-component-keeps-head-and-remaps-file
;; a component created on a branch must merge into main as a VALID head:
;; its main instance keeps component-id/main-instance and its
@ -796,6 +824,95 @@
(t/is (= "bar" (get-in data' [:plugin-data :my-plugin "foo"])))
(t/is (= "20px" (get-in data' [:options :base-font-size])))))
;; --- tokens source
(def ^:private main-file-id (uuid/custom 0 100))
(def ^:private lib-a (uuid/custom 0 101))
(def ^:private lib-b (uuid/custom 0 102))
(defn- source-data
"File-level data of main's file whose tokens source is `source` (nil =
the file itself)."
[source]
(cond-> (assoc (file-level-data) :id main-file-id)
(some? source) (assoc :tokens-source source)))
(t/deftest compute-merge-tokens-source-one-side
(let [base (source-data nil)
branch (source-data lib-a)
r (bm/compute-merge base base branch :branch->main)
c (first (:changes r))]
(t/is (= {:added 0 :modified 1 :deleted 0 :conflicts 0} (:stats r)))
(t/is (= :tokens-source (:kind c)))
(t/is (= :tokens-source (:id c)))
(t/is (= "tokens-source//tokens-source" (:key c)))))
(t/deftest compute-changes-tokens-source-one-side
(t/testing "the branch points the file at a library"
(let [base (source-data nil)
branch (source-data lib-a)
{:keys [changes unsupported]} (bm/compute-changes base base branch)]
(t/is (empty? unsupported))
(t/is (= [{:type :set-tokens-source :file-id main-file-id :library-id lib-a}] changes))
(t/is (= lib-a (:tokens-source (cfc/process-changes base changes))))))
(t/testing "the branch points the file back at itself"
(let [base (source-data lib-a)
branch (source-data nil)
{:keys [changes]} (bm/compute-changes base base branch)]
(t/is (= [{:type :set-tokens-source :file-id main-file-id :library-id nil}] changes))
(t/is (not (contains? (cfc/process-changes base changes) :tokens-source)))))
(t/testing "main's change stays when the branch leaves the source alone"
(let [base (source-data nil)
main (source-data lib-a)]
(t/is (empty? (:changes (bm/compute-merge base main base :branch->main))))
(t/is (empty? (:changes (bm/compute-changes base main base)))))))
(t/deftest compute-merge-tokens-source-self-id-reads-as-self
;; a source naming the file itself is the same as no source
(let [base (source-data nil)
branch (source-data main-file-id)]
(t/is (empty? (:changes (bm/compute-merge base base branch :branch->main))))
(t/is (empty? (:changes (bm/compute-changes base base branch))))))
(t/deftest tokens-source-merges-after-branch-remap
;; a branch that named its own file id merges into main as "self"
(let [branch-id (uuid/custom 0 103)
base (source-data lib-a)
branch (-> (source-data nil)
(assoc :id branch-id :tokens-source branch-id)
(bm/remap-refs {branch-id main-file-id}))
{:keys [changes]} (bm/compute-changes base base branch)]
(t/is (= [{:type :set-tokens-source :file-id main-file-id :library-id nil}] changes))
(t/is (not (contains? (cfc/process-changes base changes) :tokens-source)))))
(t/deftest compute-merge-tokens-source-conflict
(let [base (source-data nil)
main (source-data lib-a)
branch (source-data lib-b)
r (bm/compute-merge base main branch :branch->main)
c (first (:conflicts r))
apply* (fn [resolutions]
(:tokens-source
(cfc/process-changes main (:changes (bm/compute-changes base main branch resolutions)))))]
(t/is (= 1 (-> r :stats :conflicts)))
(t/is (= {:kind :tokens-source :id :tokens-source :reason :modify-modify
:base nil :main lib-a :branch lib-b}
(select-keys c [:kind :id :reason :base :main :branch])))
(t/is (bm/conflict-resolved? c :branch))
(t/testing "unresolved: main keeps its source"
(t/is (= lib-a (apply* {}))))
(t/testing "resolved to the branch: the branch's source lands"
(t/is (= lib-b (apply* {:tokens-source :branch}))))))
(t/deftest compute-changes-tokens-source-update-from-main
(let [base (source-data nil)
main (source-data lib-a)
r (bm/compute-merge base main base :main->branch)
{:keys [changes unsupported]} (bm/compute-changes base base main {} r)]
(t/is (= [:tokens-source] (map :kind (:changes r))))
(t/is (empty? unsupported))
(t/is (= lib-a (:tokens-source (cfc/process-changes base changes))))))
;; --- components
(t/deftest compute-changes-component-add

8
frontend/src/app/main/ui/workspace/sidebar/branch_review/common.cljs
View File

@ -69,7 +69,8 @@
:token-set-order i/tokens
:token-theme i/tokens
:token-active-themes i/tokens
:token-active-sets i/tokens})
:token-active-sets i/tokens
:tokens-source i/tokens})
(def ^:private shape-type->icon
{:frame "board" :rect "rectangle" :line "rectangle" :circle "ellipse"
@ -114,7 +115,8 @@
:token-set-order "workspace.branches.review.type.token-set-order"
:token-theme "workspace.branches.review.type.theme"
:token-active-themes "workspace.branches.review.type.active-themes"
:token-active-sets "workspace.branches.review.type.active-sets"})
:token-active-sets "workspace.branches.review.type.active-sets"
:tokens-source "workspace.branches.review.type.tokens-source"})
(def ^:private shape-type->label
{:frame "workspace.branches.compare.shape.board"
@ -308,7 +310,7 @@
(or (= :component kind) (:component? item) (:component-copy? item))
:components
(contains? #{:token :token-set :token-set-rename :token-set-order :token-theme
:token-active-themes :token-active-sets} kind)
:token-active-themes :token-active-sets :tokens-source} kind)
:tokens
(contains? #{:color} kind) :colour
(contains? #{:typography} kind) :copy

4
frontend/translations/en.po
View File

@ -11623,6 +11623,10 @@ msgstr "Active themes"
msgid "workspace.branches.review.type.active-sets"
msgstr "Active token sets"
#: src/app/main/ui/workspace/sidebar/branch_review/common.cljs
msgid "workspace.branches.review.type.tokens-source"
msgstr "Tokens source"
#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs
msgid "workspace.branches.review.facts.contents"
msgstr "Contents"