Skip to content

Commit 77371bc

Browse files
committed
docs: custom node skipping musings [skip ci]
Maybe we'll finally get to #70?
1 parent b7c7693 commit 77371bc

File tree

1 file changed

+198
-0
lines changed

1 file changed

+198
-0
lines changed

doc/design/custom-node-skipping.adoc

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
= Custom node skipping
2+
:toc:
3+
4+
== Status ==
5+
Initial musings, first draft.
6+
7+
== The Idea
8+
Currently rewrite-clj automatically skips whitespace and comments when navigating through the zipper with `left`,`right`,`down`,`up`, `next`, `prev` etc.
9+
10+
It would be nice if rewrite-clj could be asked to also skip reader discards (unevals), maybe optionally `(comment ...)` forms, and maybe other user-imagined skip scenarios.
11+
12+
The https://github.com/clj-commons/rewrite-clj/issues/70[request to skip unevals nodes] was raised a very long time ago.
13+
14+
== Personal Impetus ==
15+
I was working on MrAnderson and found, like many do, the skipping of uneval nodes awkward.
16+
I started to experiment with handling this generically in MrAnderson, but then thought maybe we could just finally handle this here in rewrite-clj for everyone.
17+
Hence this little design note.
18+
19+
== Code Snippet Notes
20+
For any code snippet, assume:
21+
[source,clojure]
22+
----
23+
(require '[rewrite-clj.zip :as z])
24+
----
25+
26+
== In the specific
27+
Let's look at the specific perceived common use cases.
28+
29+
=== Skipping whitespace and comments
30+
This is already implemented as hardcoded skip alg, would become an overrideable default.
31+
32+
=== Also skipping reader discards
33+
Comments, whitespace and unevals can be tested via `z/sexpr-able?`.
34+
So, to add in skipping unevals, we'd want to skip every node that is not `z/sepxr-able?`.
35+
36+
[source,clojure]
37+
----
38+
(a b c #_#_ skip1 skip2 d #_ (skip3 #_ skip4) e f)
39+
----
40+
41+
For this skip scenario, we effectively see `(a ...)` `a` `b` `c` `d` `e` `f` nodes when moving through the tree.
42+
43+
If we were to navigate down into, for example, `skip3` via `+*+` move fns, what would then be the effect of non-`+*+` move fns from that zipper node location?
44+
45+
To me `next` and `prev` feel straightforward:
46+
47+
- `next` -> `e`
48+
- `prev` -> `d`
49+
50+
But what about?:
51+
52+
- `right` -> `nil`? there is no non skipped right because we are in a skipped node
53+
- `left` -> `nil`? there is no non skipped left because we are in a skipped node
54+
- `up` -> `nil`?
55+
- `down` -> `nil`
56+
57+
So maybe if we are inside a skipped node all but `next` and `prev` should return `nil`?
58+
Would it be more helpful if they threw?
59+
Maybe.
60+
They are used internally by other fns.
61+
62+
If we are at a skipped node root, for example at `+#_#_ skip1 skip2+`, I think we can operate almost as per normal:
63+
64+
- `right` -> `d`
65+
- `left` -> `c`
66+
- `up` -> `(a ...)`
67+
- `down` -> `nil` nothing unskipped in this direction.
68+
69+
Logically, we can check if we are in a skipped node by:
70+
71+
- if an ancestor node is skipped, we are inside a skipped node
72+
- else if current node is skipped we are at a skipped node root
73+
- else node is not skipped
74+
75+
=== Also skipping comment forms
76+
In the same theme, a user might want to also skip any list that starts with `comment`.
77+
78+
This is a bit interesting.
79+
We'd also want to skip any whitespace, comments, unevals before the `comment` symbol.
80+
81+
[source,clojure]
82+
----
83+
foo
84+
(
85+
comment (+ 1 2 3))
86+
bar
87+
----
88+
89+
For this skip scenario, movement fns would see `foo` and `bar`.
90+
91+
== In the generic
92+
Whitespace and comment nodes are simple.
93+
They are not container nodes; they are always leaf nodes.
94+
95+
What generic affect would excluding container nodes have?
96+
97+
Let's explore with an example:
98+
99+
[source,clojure]
100+
----
101+
(a b c)
102+
[x y z [d e f]]
103+
(1 2 3 (4 [5 6 [7 8] 9 (10 11 [99 100])]))
104+
----
105+
106+
If I wanted to skip everything but vectors, what would I expect?
107+
108+
My first unskipped node would be `[x y z [d e f]]`.
109+
A `right` would return `nil`
110+
A `next` would return `nil`
111+
A `down` would move us to `[d e f]`, but a subsequent `down` would return `nil`.
112+
113+
So is this what the user really wants and/or expects?
114+
Would the user have expected to see the nested vectors `[5...]` `[7...]` `[99...]`?
115+
Is this, in the generic, at all useful?
116+
117+
Note that we already have `prewalk` and `postwalk` which could be better chandidates for some types of use cases, like "I want to visit every vector".
118+
119+
== Insertions
120+
I think we are probably fine here, but worth a think.
121+
We'll just continue with the strategy rewrite-clj has taken for comments.
122+
Existing behaviour:
123+
124+
[source,clojure]
125+
----
126+
(-> "(;; comment\na b c)"
127+
z/of-string
128+
(z/insert-child 'new)
129+
z/root-string)
130+
;; => "(new ;; comment\na b c)"
131+
132+
(-> "(a b c ;; commment\n)"
133+
z/of-string
134+
(z/append-child 'new)
135+
z/root-string)
136+
;; => "(a b c ;; commment\n new)"
137+
----
138+
139+
== Deletion
140+
The `z/remove` fn is whitespace aware.
141+
Internally it uses `z/right` `z/rightmost?` and `z/leftmost?`.
142+
Hmm... I don't think we want these tests and movements to be affected by skip behaviour.
143+
144+
== Paredit API
145+
Hmmm... have to take a look and see what makes sense.
146+
I don't think slurp and barf, for example, should be affected by skip behaviour.
147+
148+
== Internal vs External Skipping
149+
So maybe our current default skip behaviour happens to match whitespace skip behaviour, plus `+;+` comments.
150+
And we might need that whitespace skip behaviour to support internal functions, regardless of the skip behaviour a user chooses.
151+
We'd have to look at each internal usage case by case.
152+
153+
== Sub trees
154+
What about operating on a subtree?
155+
When isolating work to a subtree within a skipped node, do we need to remember we are working within a skipped node?
156+
Probably? Or maybe optionally?
157+
158+
== Performance
159+
All these extra checks will have a cost.
160+
I think we should take rough measures for the common use cases.
161+
We should work to not incur any significant extra penalty if users want to stick with current skip behaviour.
162+
163+
== Expressing skip behaviour
164+
We were thinking it would be expressed as an option on zipper creation and remain unchanged for the life of the zipper.
165+
We currently have an `auto-resolve` option that accepts a function.
166+
We were thinking of a `skip-node?` predicate, it would accept a zipper `zloc` as its single argument.
167+
168+
Here's a skip-node? predicate fn I was experimenting with:
169+
170+
[source,clojure]
171+
----
172+
(defn- skip-uninteresting-pred [zloc]
173+
(z/find zloc z/up* (fn [zloc]
174+
;; skip whitespace, comments, unevals
175+
(or (not (z/sexpr-able? zloc))
176+
;; skip (comment ...) forms
177+
(and (z/list? zloc)
178+
(when-let [first-child (some-> zloc
179+
z/down*
180+
(z/find z/right* z/sexpr-able?))]
181+
(and (n/symbol-node? (z/node first-child))
182+
(= 'comment (z/sexpr first-child)))))))))
183+
----
184+
This is entirely exploratory, experimental and unoptimized.
185+
I'm not sure of much yet.
186+
If we take the a similar approach to the above, not sure if rewrite-clj will handle the search upward or if that will be up to the predicate.
187+
Or maybe checking if we are within a skipped node will be handled through some other mechanism.
188+
189+
Alternatives:
190+
191+
- a `skip-to-node?` (or other named) predicate which would express the inverse of `skip-node?`.
192+
- hard code and accept common use cases only, ex. `:skip-node-strategies [:whitespace :comment :comment-form :uneval]`.
193+
I think the flexibility of a predicate makes more sense, and we can document examples.
194+
195+
== Other thougths
196+
Is there some key concept I am missing?
197+
Should we somehow be separating navigation from selection?
198+
Or treating containers differently than leaf nodes?

0 commit comments

Comments
 (0)