BetterThanTomorrow
discard reader symbol (#_) gets caught in Slurp Forward Sexp
noticed that Slurp Sexp Forward regards the preprocessor discard reader macro (#_) as an sexp. So starting with
(do|)
#_(def a :a)
(def b :b)
by doing Slurp Sexp Forward, I can get
(do|
#_)(def a :a)
(def b :b)
Slurping forward again a space is created after the #_.
(do|
#_ (def a :a))
(def b :b)
Something similar happens with Select Forward Sexp and with (move) Forward Sexp.
It could be argued that the discard reader should be considered natural to select the reader without the discard symbol. But other symbols (like inline function macro - #(identity) ) are considered part of the sexp they are attached to (for slurp sexp forward as well as selection and navigation). For example in (def ^:dynamic my-var 10) the ^:dynamic is considered part of my-var for selection.
https://github.com/user-attachments/assets/699466e9-4790-4d0f-b32c-a6a1911a0224
This is discussed here: https://clojurians.slack.com/archives/CBE668G4R/p1753310049089449
In that thread, Phill points out that "evaluate current form" #_|42 or #_42| might be related. If I hop on this, I should see if I can write a unit test for that first, if there isn't a test now.
There's a straightforward fix for this (below) but with this change it skips past the ignored expression completely and goes to the next expression. If there's no next expression the parens stay where they were.
So
(do|)
#_(def a :a)
(def b :b)
becomes
(do|
#_(def a :a)
(def b :b))
with the do paren moved to just after the :b. This seems OK-ish, but I'm not sure. I'll play with it some.
diff --git a/src/cursor-doc/paredit.ts b/src/cursor-doc/paredit.ts
index 81f088f77..f7c343cc1 100644
--- a/src/cursor-doc/paredit.ts
+++ b/src/cursor-doc/paredit.ts
@@ -909,7 +909,7 @@ function forwardSlurpSexpEdits(doc: EditableDocument, start: number): ModelEdit<
const wsStartOffset = wsInsideCursor.offsetStart;
cursor.upList();
const wsOutSideCursor = cursor.clone();
- if (cursor.forwardSexp(true, true)) {
+ if (cursor.forwardSexp(true, true, true)) {
wsOutSideCursor.forwardWhitespace(false);
const wsEndOffset = wsOutSideCursor.offsetStart;
const newCloseOffset = cursor.offsetStart;
https://github.com/user-attachments/assets/79fbe45c-3bf3-4ed1-bd12-03787656fb27
Thanks for reporting. There are some conflicting constraints in play here.
- We want consistency across commands, so moving or selecting a sexpression forward should move past or select forward the same sexpression as slurp forward would slurp.
- The Current Form should be consistent with what we move past/select/slurp. So with the cursor after the ignored form moving/selecting/slurping backwards should involve the same form as selecting the current form would select (or what would be evaluated if we evaluate the current form).
- There is a special requirement of evaluating the current form when it is ignored: It should still evaluate the current form.
We sometimes want to treat the ignored form as one form, and sometimes as two forms. Something has to give, and we have so far accepted that we will slurp the ignore marker separately from the form it marks.
What we could consider is to expose the behaviour of skipping ignored forms to the slurp command. And then people can configure keyboard shortcuts that slurps like in your example there... Not sure what it entails, but it should be entirely doable.
This is video/diff getting closer to what I had originally thought the behavior would be. With Slurp Forward Sexp, it moves to the end of the ignored form, including it. So it's unlike the small change to paredit.ts that I included above in that the ignored form is treated as if it is an sexp. (This might get into philosophy, but it feels like an ignored sexp is still an sexp.)
https://github.com/user-attachments/assets/116ae725-eb1a-47a2-b502-f300a80a2ad8
diff --git a/src/cursor-doc/paredit.ts b/src/cursor-doc/paredit.ts
index 81f088f77..f7c343cc1 100644
--- a/src/cursor-doc/paredit.ts
+++ b/src/cursor-doc/paredit.ts
@@ -909,7 +909,7 @@ function forwardSlurpSexpEdits(doc: EditableDocument, start: number): ModelEdit<
const wsStartOffset = wsInsideCursor.offsetStart;
cursor.upList();
const wsOutSideCursor = cursor.clone();
- if (cursor.forwardSexp(true, true)) {
+ if (cursor.forwardSexp(true, true, true)) {
wsOutSideCursor.forwardWhitespace(false);
const wsEndOffset = wsOutSideCursor.offsetStart;
const newCloseOffset = cursor.offsetStart;
diff --git a/src/cursor-doc/token-cursor.ts b/src/cursor-doc/token-cursor.ts
index caf3e87fd..bad832e3b 100644
--- a/src/cursor-doc/token-cursor.ts
+++ b/src/cursor-doc/token-cursor.ts
@@ -259,11 +259,16 @@ export class LispTokenCursor extends TokenCursor {
break;
case 'ignore':
if (skipIgnoredForms) {
+ // skip by the #_ and the following sexp; return out if parens are balanced
this.next();
this.forwardSexp(skipComments, skipMetadata, skipIgnoredForms);
+ if (stack.length <= 0) {
+ return true;
+ }
break;
}
// eslint-disable-next-line no-fallthrough
+
case 'id':
case 'lit':
case 'kw':
Of course, this doesn't address navigation or selection yet, or even Barf Forward Sexp. They should be straightforward, so I can work on adding these. For now, I'm just wondering if this is the right approach.
I'm pretty sure it doesn't change evaluation.
I added forwardBarfSexp, but I started looking at move sexp up/down. The released code works fine without a reader discard, but with the discard (#_), things break. If I start with
(do
| (+ 1 1)
(+ 2 2)
#_4
(def a :a))
The first time I do MoveSexpDown, I get, as expected
(do
(+ 2 2)
| (+ 1 1)
#_4
(def a :a))
But then I get something confusing
(do
(+ 2 2)
4
#_|(+ 1 1)
(def a :a))
The discard reader is annoying! I'll see if I can address this while I'm hacking around.
This is the idea I had for keeping the discount/ignore token while doing MoveSexpDown and MoveSexpUp. My changes have a few bugs, but I figured I'd share if there is any feedback
https://github.com/user-attachments/assets/a2280baa-1856-4c5a-a11b-9b3912227ee3
Playing around, I see that the discard symbol is also ignored by
- Drag Sexp Backward Down (ignore part of target sexp is ignored when dragging)
- Drag Sexp Backward Up (ignore part of target sexp is ignored when dragging)
- Drag Sexp Foreward Down (ignore part of target sexp is ignored when dragging)
- Drag Sexp Foreward Up (ignore part of target sexp is ignored when dragging)
- Kill/Delete Sexp Backward (ignore part of target sexp is ignored when deleting)
- Kill/Delete Sexp Foreward (ignore token is deleted instead of whole sexp)
- Split Sexp (ignore token is treated as a sexp if cursor is after it, eg: [:a :b #_|:c])
- Splice Sexp (ignore token on target list is kept, so it applies to the former first element #[:a :b |:c] -> #:a :b |:c)
- Transpose (ignore token is treated as sexp and transposed
- Expand Selection (ignore token is not considered as part of an sexp when expanding)
- Shrink Selection (ignore token is not considered as part of an sexp when shrinking)
- Wrap Around * (ignore token is considered as an sexp of its own)
I think these are doable to address.