Drag sexp comments 3073#3178
Conversation
A new `extendRangeBackwardOverPrecedingLineComments` helper walks backward from a range start over any contiguous line comments with no intervening blank line. Not wired up yet; will be used by the drag-sexp commands so comments travel with their associated form. Refs BetterThanTomorrow#3073
Use `extendRangeBackwardOverPrecedingLineComments` when computing the text ranges to swap in `dragSexprBackward`, so a comment attached to the form above travels with it. Cursor position is derived from the new extended ranges. Refs BetterThanTomorrow#3073
Mirror the backward change: use `extendRangeBackwardOverPrecedingLineComments` to compute the swap ranges in `dragSexprForward`. The cursor formula uses the original ranges' ends (extensions only affect starts), so it needs no change. Refs BetterThanTomorrow#3073
Skip leading spaces/tabs on the form's own line before checking for a preceding newline, so nested forms inside containers pick up their comment lines the same way top-level forms do. Refs BetterThanTomorrow#3073
✅ Deploy Preview for calva-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Cool. Cases that bite me at times is when the comment sits directly below or to the right: (+ 1 2)
;=> 3
(+ 1 2) ; => 3Often as a result of Evaluate to Comment. |
…ments Replace `extendRangeBackwardOverPrecedingLineComments` and `formAttachedToCommentLineAt` with `extendRangeOverAttachedComments` and `formAttachedToCommentAt`, sharing a small `lineInfoAt` / `prevLine` / `nextLine` utility. The unified range extender also covers same-line trailing `;...` comments and comment lines below the form that are not leading a following form (terminated by a blank line or EOF). The comment-lookup helper now resolves trailing-annotation cursors by falling back to the form above when no form follows without a gap. Drag functions are simplified to pass `baseRange` (the bare form) to navigation and `currentRange` (the extended logical form) to the edit.
…osg/calva into fix/drag-sexp-comments-3073
There was a problem hiding this comment.
Pull request overview
Adjusts Calva’s Paredit “drag sexp” behavior so that forms move together with their associated line comments (leading, trailing inline, and certain trailing comment blocks), addressing the adjacent-comment reordering problem in #3073.
Changes:
- Extend drag ranges to include “attached” comments and allow initiating drag from comment lines by resolving the attached form first.
- Add unit tests covering drag behavior with leading/trailing comments and blank-line association rules.
- Update keybinding
whenclauses to allow drag commands while the cursor is in comments; add changelog entry for #3073.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/extension-test/unit/cursor-doc/paredit-test.ts | Adds unit tests for drag/comment attachment scenarios (but currently includes an unused import). |
| src/cursor-doc/paredit.ts | Implements comment-aware range extension and comment→form attachment resolution for drag operations. |
| package.json | Enables drag keybindings in comment contexts by removing !calva:cursorInComment. |
| CHANGELOG.md | Adds an Unreleased entry referencing issue #3073. |
| const baseRange = | ||
| formAttachedToCommentAt(doc, right) ?? currentSexpsRange(doc, cursor, right, usePairs, config); |
There was a problem hiding this comment.
When the cursor is on a line comment, baseRange falls back to currentSexpsRange if formAttachedToCommentAt returns null. Because the token cursor’s whitespace skipping treats comments as whitespace by default, this can end up dragging a neighboring form even when the comment is intentionally not attached (e.g. separated by a blank line). To preserve the “blank line breaks association” semantics, consider returning early (no-op) when offset is on a comment line but formAttachedToCommentAt returns null.
| const baseRange = | |
| formAttachedToCommentAt(doc, right) ?? currentSexpsRange(doc, cursor, right, usePairs, config); | |
| const attachedCommentRange = formAttachedToCommentAt(doc, right); | |
| const text = doc.model.getText(0, Number.MAX_SAFE_INTEGER); | |
| const onCommentLine = isCommentLine(lineInfoAt(text, right)); | |
| if (onCommentLine && !attachedCommentRange) { | |
| return; | |
| } | |
| const baseRange = | |
| attachedCommentRange ?? currentSexpsRange(doc, cursor, right, usePairs, config); |
|
|
||
| ## [Unreleased] | ||
|
|
||
| - [Dragging sexps has weird behavior with adjacent comments](https://github.com/BetterThanTomorrow/calva/issues/3073) |
There was a problem hiding this comment.
Changelog entries for bug fixes in this repo appear to use the Fix: prefix (e.g. 2.0.578, 2.0.575). Since this PR fixes #3073, consider updating this Unreleased entry to use Fix: for consistency.
| - [Dragging sexps has weird behavior with adjacent comments](https://github.com/BetterThanTomorrow/calva/issues/3073) | |
| - Fix: [Dragging sexps has weird behavior with adjacent comments](https://github.com/BetterThanTomorrow/calva/issues/3073) |
| import * as model from '../../../cursor-doc/model'; | ||
| import * as textNotation from '../common/text-notation'; | ||
| import * as _ from 'lodash'; | ||
| import { text } from 'stream/consumers'; |
There was a problem hiding this comment.
Unused import text from stream/consumers is not referenced anywhere in this test file; this will fail lint/TS compilation. Remove the import.
| import { text } from 'stream/consumers'; |
| } | ||
| if (trailingEnds.length && (!below || isBlankLine(below))) { | ||
| hi = trailingEnds[trailingEnds.length - 1]; |
There was a problem hiding this comment.
extendRangeOverAttachedComments only attaches comment lines below the form if they’re followed by a blank line or EOF. Calva’s evaluation result comments are produced as \n;;=> … (see resultAsComment) and can be immediately followed by the next form without an extra blank line, in which case the ;;=> lines would not be moved with the form. Consider treating ;;=> (and/or ;=>) result comment lines as attached-to-previous even when another form follows.
There was a problem hiding this comment.
This is something we can consider following up with, @jramosg. Sometimes you want a tight RCF with no blank lines, and the ;=> marker is what tells us where the comment belongs.
There was a problem hiding this comment.
you're right, now is handled in trailingResultCommentBlockEnd
Just checking. Are these cases also covered? |
|
Yes, they are now covered and tested! Do you miss some test case? |
|
Asked before checking test cases 😄 . Since there was no reaction or comment I didn't know if you had maybe missed it. Things look good to me. This will be a great add! |
|
Oh sorry, I should answered before 🥲 |
This change introduces functionality to identify and extend ranges over trailing result comments in the paredit module. It adds a new helper function to detect result comment blocks and modifies the existing range extension logic to include these comments, improving the handling of comments in code.
This change introduces two test cases that verify the behavior of dragging sexp backward when a result comment follows immediately. These tests ensure that the functionality works correctly for both `;;=>` and `;=>` result comments.
Enhance the handling of result comments by introducing a function to identify result comment blocks. This ensures that trailing result comments are correctly processed when extending ranges over attached comments, improving the overall functionality of the paredit operations.
This commit introduces two test cases for the dragSexprForward function, ensuring it correctly handles result comments (both `;;=>` and `;=>`) when the next form follows immediately. This enhances the test coverage for the paredit functionality.
|
Thanks for the updates! 🙏 I'm testing the VSIX of #4c8ae16, the next one wasn't built, and it says it is adding tests anyway. Found these: Extra space inserted for the comment(comment
; a
:a|
:b
)Drag forward (comment
:b
; a
:a|
)It continues to be insertd more spaces: Drag backward (comment
; a
:a|
:b
)However, this one works fine: (comment
; a
:a|
; b
:b
)Drag forward (comment
; b
:b
; a
:a|
)“tight” RCFs(comment
:a
:b|
;=> b
:c
)Drag forward gives a warning message Overlapping ranges are not allowed. Drag backward: (comment
:b|
;=> b
:a :c
)
|
|
I hope I got the code blocks right. 😄 In fact, I am pretty sure I may have mixed something up. |
|
thanks! I will have a look |
This change introduces functions to extend ranges to include leading comments when dragging sexps. It ensures that the text manipulation correctly accounts for comments, improving the handling of ranges during drag operations.
…dentation This commit introduces tests for dragging commented forms both forward and backward without altering indentation. It ensures that comments remain attached during the drag operations, enhancing the functionality and reliability of the paredit features.
Enhance the form attachment detection by adding support for result comments. This change ensures that when a result comment is encountered, the cursor can correctly navigate to the associated form, improving the user experience during editing operations.
This update introduces several test cases to ensure that dragging forms forward and backward correctly handles tight result comments. The tests verify that comments remain attached or detached as expected during drag operations, enhancing the robustness of the drag functionality in paredit.
Screen.Recording.2026-05-05.at.09.28.51.movI think they are now fixed! |
|
This one packs a lot 😄 [:a ; a same line
; b line before
|:b] ; b same lineDrag backwards [ ; b line before
:b
:a ; a same line] ; b same lineDragging The above is a minified version of code like this: (defn- clean-list-content
"Clean content but don't double-indent nested lists that are already indented"
[content indent-spaces]
(let [indent (apply str (repeat indent-spaces " "))]
(-> content
(.replace #"^\n+" "") ; remove leading newlines
(.replace #"\n+$" "\n") ; keep single trailing newline
;; Only indent lines that don't already start with spaces (nested lists)
(.replace #"\n(?! )" (str "\n" indent))))) ; indent nested content with single space |
Now the warning message is gone, but we get: (comment
:a
:b
;=> b :c
)(The big problem here is that it comments out |
What build are you testing? I think I fixed it yesterday, and I can’t reproduce it now. It’s strange because I thought I had already fixed it before your comment. Video: |
this is really tricki, let's see what can I do. Maybe we should ignore comment form pair here? There are not blank lines so we cannot decide which form belongs [:a ; a same line
; b line before
|:b] ; b same line |
What has changed?
formAttachedToCommentAtis used if the cursor is on a comment to find the attached form. Otherwise current fncurrentSexpsRangeis used.extendRangeOverAttachedComments, so the actual drag operation moves the form together with its attached comments.Finds comments at the top, at the right and at the bottom of the form.
#_comments are not taken into account, should they?Fixes #3073
My Calva PR Checklist
I have:
devbranch. (Or have specific reasons to target some other branch.)published. (Sorry for the nagging.)[Unreleased]entry inCHANGELOG.md, linking the issue(s) that the PR is addressing.npm run prettier-format)npm run eslintbefore creating your PR, or runnpm run eslint-watchto eslint as you go).