mirror of
https://github.com/penpot/penpot.git
synced 2026-04-06 03:12:40 +02:00
🐛 Fix nil path content crash by exposing safe public API
Move nil-safety for path segment helpers to the public API layer (app.common.types.path) rather than the low-level segment namespace. Add nil-safe wrappers for get-handlers, opposite-index, get-handler-point, get-handler, handler->node, point-indices, handler-indices, next-node, append-segment, points->content, closest-point, make-corner-point, make-curve-point, split-segments, remove-nodes, merge-nodes, join-nodes, and separate-nodes. Update all frontend callers to use path/ instead of path.segment/ for these functions, removing the path.segment require from helpers, drawing, edition, tools, curve, editor and debug. Replace ad-hoc nil checks with impl/path-data coercion in all public wrapper functions in app.common.types.path. The path-data helper already handles nil by returning an empty PathData instance, which provides uniform nil-safety across all content-accepting functions. Update the path-get-points-nil-safe test to expect empty collection instead of nil, matching the new coercion behavior.
This commit is contained in:
@@ -191,19 +191,128 @@
|
||||
|
||||
(defn get-points
|
||||
"Returns points for the given content. Accepts PathData instances or
|
||||
plain segment vectors. Returns nil for nil content."
|
||||
plain segment vectors."
|
||||
[content]
|
||||
(when (some? content)
|
||||
(let [content (if (impl/path-data? content)
|
||||
content
|
||||
(impl/path-data content))]
|
||||
(segment/get-points content))))
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/get-points content)))
|
||||
|
||||
(defn calc-selrect
|
||||
"Calculate selrect from a content. The content can be in a PathData
|
||||
instance or plain vector of segments."
|
||||
[content]
|
||||
(segment/content->selrect content))
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/content->selrect content)))
|
||||
|
||||
(defn get-handlers
|
||||
"Retrieve a map where for every point will retrieve a list of the
|
||||
handlers that are associated with that point.
|
||||
point -> [[index, prefix]]"
|
||||
[content]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/get-handlers content)))
|
||||
|
||||
(defn get-handler-point
|
||||
"Given a content, segment index and prefix, get a handler point."
|
||||
[content index prefix]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/get-handler-point content index prefix)))
|
||||
|
||||
(defn get-handler
|
||||
"Given a segment (command map) and a prefix, returns the handler
|
||||
coordinate map {:x ... :y ...} from its params, or nil when absent."
|
||||
[command prefix]
|
||||
(segment/get-handler command prefix))
|
||||
|
||||
(defn handler->node
|
||||
"Given a content, index and prefix, returns the path node (anchor
|
||||
point) that the handler belongs to."
|
||||
[content index prefix]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/handler->node content index prefix)))
|
||||
|
||||
(defn opposite-index
|
||||
"Calculates the opposite handler index given a content, index and
|
||||
prefix."
|
||||
[content index prefix]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/opposite-index content index prefix)))
|
||||
|
||||
(defn point-indices
|
||||
"Returns the indices of all segments whose endpoint matches point."
|
||||
[content point]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/point-indices content point)))
|
||||
|
||||
(defn handler-indices
|
||||
"Returns [[index prefix] ...] of all handlers associated with point."
|
||||
[content point]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/handler-indices content point)))
|
||||
|
||||
(defn next-node
|
||||
"Calculates the next node segment to be inserted when drawing."
|
||||
[content position prev-point prev-handler]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/next-node content position prev-point prev-handler)))
|
||||
|
||||
(defn append-segment
|
||||
"Appends a segment to content, accepting PathData or plain vector."
|
||||
[content segment]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/append-segment content segment)))
|
||||
|
||||
(defn points->content
|
||||
"Given a vector of points generate a path content."
|
||||
[points & {:keys [close]}]
|
||||
(segment/points->content points :close close))
|
||||
|
||||
(defn closest-point
|
||||
"Returns the closest point in the path to position, at a given precision."
|
||||
[content position precision]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/closest-point content position precision)))
|
||||
|
||||
(defn make-corner-point
|
||||
"Changes the content to make a point a corner."
|
||||
[content point]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/make-corner-point content point)))
|
||||
|
||||
(defn make-curve-point
|
||||
"Changes the content to make a point a curve."
|
||||
[content point]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/make-curve-point content point)))
|
||||
|
||||
(defn split-segments
|
||||
"Given a content, splits segments between points with new segments."
|
||||
[content points value]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/split-segments content points value)))
|
||||
|
||||
(defn remove-nodes
|
||||
"Removes the given points from content, reconstructing paths as needed."
|
||||
[content points]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/remove-nodes content points)))
|
||||
|
||||
(defn merge-nodes
|
||||
"Reduces contiguous segments at the given points to a single point."
|
||||
[content points]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/merge-nodes content points)))
|
||||
|
||||
(defn join-nodes
|
||||
"Creates new segments between points that weren't previously connected."
|
||||
[content points]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/join-nodes content points)))
|
||||
|
||||
(defn separate-nodes
|
||||
"Removes the segments between the given points."
|
||||
[content points]
|
||||
(let [content (impl/path-data content)]
|
||||
(segment/separate-nodes content points)))
|
||||
|
||||
(defn- calc-bool-content*
|
||||
"Calculate the boolean content from shape and objects. Returns plain
|
||||
|
||||
@@ -273,8 +273,8 @@
|
||||
(t/is (= result2 result3))))
|
||||
|
||||
(t/deftest path-get-points-nil-safe
|
||||
(t/testing "path/get-points returns nil for nil content without throwing"
|
||||
(t/is (nil? (path/get-points nil))))
|
||||
(t/testing "path/get-points returns empty for nil content without throwing"
|
||||
(t/is (empty? (path/get-points nil))))
|
||||
(t/testing "path/get-points returns correct points for valid content"
|
||||
(let [content (path/content sample-content)
|
||||
points (path/get-points content)]
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
[app.common.geom.shapes.flex-layout :as gslf]
|
||||
[app.common.geom.shapes.grid-layout :as gslg]
|
||||
[app.common.types.container :as ctn]
|
||||
[app.common.types.path.segment :as path.segment]
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.shape :as cts]
|
||||
[app.common.types.shape-tree :as ctst]
|
||||
[app.common.types.shape.layout :as ctl]
|
||||
@@ -33,7 +33,7 @@
|
||||
(update [_ state]
|
||||
(let [objects (dsh/lookup-page-objects state)
|
||||
content (dm/get-in state [:workspace-drawing :object :content])
|
||||
position (path.segment/get-handler-point content 0 nil)
|
||||
position (path/get-handler-point content 0 nil)
|
||||
|
||||
frame-id (->> (ctst/top-nested-frame objects position)
|
||||
(ctn/get-first-valid-parent objects) ;; We don't want to change the structure of component copies
|
||||
@@ -65,8 +65,8 @@
|
||||
(fn [object]
|
||||
(let [points (-> (::points object)
|
||||
(conj point))
|
||||
content (path.segment/points->content points)
|
||||
selrect (path.segment/content->selrect content)
|
||||
content (path/points->content points)
|
||||
selrect (path/calc-selrect content)
|
||||
points' (grc/rect->points selrect)]
|
||||
(-> object
|
||||
(assoc ::points points)
|
||||
@@ -82,8 +82,8 @@
|
||||
(update-in state [:workspace-drawing :object]
|
||||
(fn [{:keys [::points] :as shape}]
|
||||
(let [points (ups/simplify points simplify-tolerance)
|
||||
content (path.segment/points->content points)
|
||||
selrect (path.segment/content->selrect content)
|
||||
content (path/points->content points)
|
||||
selrect (path/calc-selrect content)
|
||||
points (grc/rect->points selrect)]
|
||||
|
||||
(-> shape
|
||||
|
||||
@@ -13,7 +13,6 @@
|
||||
[app.common.types.container :as ctn]
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.path.helpers :as path.helpers]
|
||||
[app.common.types.path.segment :as path.segment]
|
||||
[app.common.types.shape :as cts]
|
||||
[app.common.types.shape-tree :as ctst]
|
||||
[app.common.types.shape.layout :as ctl]
|
||||
@@ -64,7 +63,7 @@
|
||||
{:keys [last-point prev-handler]}
|
||||
(get-in state [:workspace-local :edit-path id])
|
||||
|
||||
segment (path.segment/next-node shape position last-point prev-handler)]
|
||||
segment (path/next-node shape position last-point prev-handler)]
|
||||
(assoc-in state [:workspace-local :edit-path id :preview] segment)))))
|
||||
|
||||
(defn add-node
|
||||
@@ -99,7 +98,7 @@
|
||||
prefix (or prefix :c1)
|
||||
position (or position (path.helpers/segment->point (nth content (dec index))))
|
||||
|
||||
old-handler (path.segment/get-handler-point content index prefix)
|
||||
old-handler (path/get-handler-point content index prefix)
|
||||
|
||||
handler-position (cond-> (gpt/point x y)
|
||||
shift? (path.helpers/position-fixed-angle position))
|
||||
@@ -148,7 +147,7 @@
|
||||
ptk/WatchEvent
|
||||
(watch [_ state stream]
|
||||
(let [content (st/get-path state :content)
|
||||
handlers (-> (path.segment/get-handlers content)
|
||||
handlers (-> (path/get-handlers content)
|
||||
(get position))
|
||||
|
||||
[idx prefix] (when (= (count handlers) 1)
|
||||
|
||||
@@ -11,7 +11,6 @@
|
||||
[app.common.geom.point :as gpt]
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.path.helpers :as path.helpers]
|
||||
[app.common.types.path.segment :as path.segment]
|
||||
[app.main.data.changes :as dch]
|
||||
[app.main.data.helpers :as dsh]
|
||||
[app.main.data.workspace.edition :as dwe]
|
||||
@@ -74,8 +73,8 @@
|
||||
|
||||
(defn modify-content-point
|
||||
[content {dx :x dy :y} modifiers point]
|
||||
(let [point-indices (path.segment/point-indices content point) ;; [indices]
|
||||
handler-indices (path.segment/handler-indices content point) ;; [[index prefix]]
|
||||
(let [point-indices (path/point-indices content point) ;; [indices]
|
||||
handler-indices (path/handler-indices content point) ;; [[index prefix]]
|
||||
|
||||
modify-point
|
||||
(fn [modifiers index]
|
||||
@@ -258,10 +257,10 @@
|
||||
points (path/get-points content)
|
||||
|
||||
point (-> content (nth (if (= prefix :c1) (dec index) index)) (path.helpers/segment->point))
|
||||
handler (-> content (nth index) (path.segment/get-handler prefix))
|
||||
handler (-> content (nth index) (path/get-handler prefix))
|
||||
|
||||
[op-idx op-prefix] (path.segment/opposite-index content index prefix)
|
||||
opposite (path.segment/get-handler-point content op-idx op-prefix)]
|
||||
[op-idx op-prefix] (path/opposite-index content index prefix)
|
||||
opposite (path/get-handler-point content op-idx op-prefix)]
|
||||
|
||||
(streams/drag-stream
|
||||
(rx/concat
|
||||
@@ -344,7 +343,7 @@
|
||||
(-> state
|
||||
(assoc-in [:workspace-local :edit-path id :old-content] content)
|
||||
(st/set-content (-> content
|
||||
(path.segment/split-segments #{from-p to-p} t)
|
||||
(path/split-segments #{from-p to-p} t)
|
||||
(path/content))))))
|
||||
|
||||
ptk/WatchEvent
|
||||
|
||||
@@ -9,15 +9,14 @@
|
||||
[app.common.geom.point :as gpt]
|
||||
[app.common.math :as mth]
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.path.helpers :as path.helpers]
|
||||
[app.common.types.path.segment :as path.segment]))
|
||||
[app.common.types.path.helpers :as path.helpers]))
|
||||
|
||||
(defn append-node
|
||||
"Creates a new node in the path. Usually used when drawing."
|
||||
[shape position prev-point prev-handler]
|
||||
(let [segment (path.segment/next-node (:content shape) position prev-point prev-handler)]
|
||||
(let [segment (path/next-node (:content shape) position prev-point prev-handler)]
|
||||
(-> shape
|
||||
(update :content path.segment/append-segment segment)
|
||||
(update :content path/append-segment segment)
|
||||
(path/update-geometry))))
|
||||
|
||||
(defn angle-points [common p1 p2]
|
||||
@@ -61,11 +60,11 @@
|
||||
[content index prefix match-distance? match-angle? dx dy]
|
||||
|
||||
(let [[cx cy] (path.helpers/prefix->coords prefix)
|
||||
[op-idx op-prefix] (path.segment/opposite-index content index prefix)
|
||||
[op-idx op-prefix] (path/opposite-index content index prefix)
|
||||
|
||||
node (path.segment/handler->node content index prefix)
|
||||
handler (path.segment/get-handler-point content index prefix)
|
||||
opposite (path.segment/get-handler-point content op-idx op-prefix)
|
||||
node (path/handler->node content index prefix)
|
||||
handler (path/get-handler-point content index prefix)
|
||||
opposite (path/get-handler-point content op-idx op-prefix)
|
||||
|
||||
[ocx ocy] (path.helpers/prefix->coords op-prefix)
|
||||
[odx ody] (calculate-opposite-delta node handler opposite match-angle? match-distance? dx dy)
|
||||
|
||||
@@ -8,7 +8,6 @@
|
||||
(:require
|
||||
[app.common.data.macros :as dm]
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.path.segment :as path.segment]
|
||||
[app.main.data.changes :as dch]
|
||||
[app.main.data.helpers :as dsh]
|
||||
[app.main.data.workspace.edition :as dwe]
|
||||
@@ -59,7 +58,7 @@
|
||||
(process-path-tool
|
||||
(when point #{point})
|
||||
(fn [content points]
|
||||
(reduce path.segment/make-corner-point content points)))))
|
||||
(reduce path/make-corner-point content points)))))
|
||||
|
||||
(defn make-curve
|
||||
([]
|
||||
@@ -68,22 +67,22 @@
|
||||
(process-path-tool
|
||||
(when point #{point})
|
||||
(fn [content points]
|
||||
(reduce path.segment/make-curve-point content points)))))
|
||||
(reduce path/make-curve-point content points)))))
|
||||
|
||||
(defn add-node []
|
||||
(process-path-tool (fn [content points] (path.segment/split-segments content points 0.5))))
|
||||
(process-path-tool (fn [content points] (path/split-segments content points 0.5))))
|
||||
|
||||
(defn remove-node []
|
||||
(process-path-tool path.segment/remove-nodes))
|
||||
(process-path-tool path/remove-nodes))
|
||||
|
||||
(defn merge-nodes []
|
||||
(process-path-tool path.segment/merge-nodes))
|
||||
(process-path-tool path/merge-nodes))
|
||||
|
||||
(defn join-nodes []
|
||||
(process-path-tool path.segment/join-nodes))
|
||||
(process-path-tool path/join-nodes))
|
||||
|
||||
(defn separate-nodes []
|
||||
(process-path-tool path.segment/separate-nodes))
|
||||
(process-path-tool path/separate-nodes))
|
||||
|
||||
(defn toggle-snap []
|
||||
(ptk/reify ::toggle-snap
|
||||
|
||||
@@ -15,7 +15,6 @@
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.path.bool :as path.bool]
|
||||
[app.common.types.path.helpers :as path.helpers]
|
||||
[app.common.types.path.segment :as path.segment]
|
||||
[app.common.types.path.subpath :as path.subpath]
|
||||
[app.main.refs :as refs]
|
||||
[app.util.color :as uc]
|
||||
@@ -124,8 +123,8 @@
|
||||
(path.bool/add-previous))
|
||||
|
||||
|
||||
sr-a (path.segment/content->selrect content-a)
|
||||
sr-b (path.segment/content->selrect content-b)
|
||||
sr-a (path/calc-selrect content-a)
|
||||
sr-b (path/calc-selrect content-b)
|
||||
|
||||
[content-a-split content-b-split] (path.bool/content-intersect-split content-a content-b sr-a sr-b)
|
||||
|
||||
|
||||
@@ -11,7 +11,6 @@
|
||||
[app.common.geom.point :as gpt]
|
||||
[app.common.types.path :as path]
|
||||
[app.common.types.path.helpers :as path.helpers]
|
||||
[app.common.types.path.segment :as path.segment]
|
||||
[app.main.data.workspace.path :as drp]
|
||||
[app.main.snap :as snap]
|
||||
[app.main.store :as st]
|
||||
@@ -251,8 +250,8 @@
|
||||
(defn- matching-handler? [content node handlers]
|
||||
(when (= 2 (count handlers))
|
||||
(let [[[i1 p1] [i2 p2]] handlers
|
||||
p1 (path.segment/get-handler-point content i1 p1)
|
||||
p2 (path.segment/get-handler-point content i2 p2)
|
||||
p1 (path/get-handler-point content i1 p1)
|
||||
p2 (path/get-handler-point content i2 p2)
|
||||
|
||||
v1 (gpt/to-vec node p1)
|
||||
v2 (gpt/to-vec node p2)
|
||||
@@ -309,7 +308,7 @@
|
||||
|
||||
handlers
|
||||
(mf/with-memo [content]
|
||||
(path.segment/get-handlers content))
|
||||
(path/get-handlers content))
|
||||
|
||||
is-path-start
|
||||
(not (some? last-point))
|
||||
@@ -331,7 +330,7 @@
|
||||
ms/mouse-position
|
||||
(mf/deps base-content zoom)
|
||||
(fn [position]
|
||||
(when-let [point (path.segment/closest-point base-content position (/ 0.01 zoom))]
|
||||
(when-let [point (path/closest-point base-content position (/ 0.01 zoom))]
|
||||
(reset! hover-point (when (< (gpt/distance position point) (/ 10 zoom)) point)))))
|
||||
|
||||
[:g.path-editor {:ref editor-ref}
|
||||
@@ -367,7 +366,7 @@
|
||||
(fn [[index prefix]]
|
||||
;; FIXME: get-handler-point is executed twice for each
|
||||
;; render, this can be optimized
|
||||
(let [handler-position (path.segment/get-handler-point content index prefix)]
|
||||
(let [handler-position (path/get-handler-point content index prefix)]
|
||||
(not= position handler-position)))
|
||||
|
||||
position-handlers
|
||||
@@ -390,7 +389,7 @@
|
||||
[:g.path-node {:key (dm/str pos-x "-" pos-y)}
|
||||
[:g.point-handlers {:pointer-events (when (= edit-mode :draw) "none")}
|
||||
(for [[hindex prefix] position-handlers]
|
||||
(let [handler-position (path.segment/get-handler-point content hindex prefix)
|
||||
(let [handler-position (path/get-handler-point content hindex prefix)
|
||||
handler-hover? (contains? hover-handlers [hindex prefix])
|
||||
moving-handler? (= handler-position moving-handler)
|
||||
matching-handler? (matching-handler? content position position-handlers)]
|
||||
|
||||
Reference in New Issue
Block a user