Improvements to cider-eval-to-comment#4018
Draft
yuhan0 wants to merge 7 commits into
Draft
Conversation
The pprint-to-comment handler accumulated stderr into the same variable as the eval value, so when nREPL response chunks interleaved (e.g. a reflection warning arriving between value chunks), the result comment would contain garbled output. Accumulate them separately and compose at on-done: whichever is present alone, or value followed by a newline-separated trimmed stderr block.
Contributor
Author
|
(sorry, something seems to have broken when i fastforwarded the last 2-3 days worth of commits - currently investigating, will mark when it's ready for review) |
Move the integer->marker coercion from the callsites into the handler itself via copy-marker, so the handler's contract accepts both types. No external facing change. Add a unit test covering the existing behavior (introduced in clojure-emacs#2607)
Add an optional REPLACE arg to the multiline-comment eval handler and to cider-pprint-form-to-comment. When set, cider-maybe-delete-multiline-comment removes the previously-inserted result region (both ;; line comments and discarded #_/(comment ...) forms) before the new result is inserted, so re-evaluating a form updates its comment in place instead of stacking. cider-maybe-insert-multiline-comment now returns the bounds of the inserted text (nil when nothing was inserted), and preserves blank lines in pretty-printed output.
yuhan0
force-pushed
the
eval-to-comment-pr
branch
from
5ee9bb8 to
b085302
Compare
Contributor
Author
|
Fixed the bug, which was due to me splitting out some WIP / personal changes from this PR - it should be ready for review now. |
yuhan0
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some improvements to
cider-eval-defun-to-commentcommands:Prevent value and stderr comments from being interleaved
#3148 introduced printing of stderr output (e.g. reflection warnings, compile errors) into the comments, however these were naively concatenated with the value strings as they streamed in, producing out-of-order or interleaved text in some cases.
Stderr is now accumulated separately and appended after the value with a newline.
Replace stale eval comments on subsequent re-evals
Easier to explain with the following gif:
Screen.Recording.2026-06-25.at.19.06.02.mov
Importantly, it only matches against the exact
cider-comment-prefix, and deletion only extends as far as the continued prefix.Open question
Do we want to introduce a separate toggle defcustom for this behavior? I personally don't see a good reason a user would prefer the current behaviour of accumulating stale results. It's also easy enough to 'bypass' by adding even a single newline between the form and the eval comment, which breaks the match detection.
That was my original plan - 2771d8b adds this defcustom (defaulting to t) and the following commit 7df655e removes it entirely making the detect-and-replace behavior always active.
Will either drop the latter commit or squash away the back-and-forth changes depending on the decision.
Also fixes
Minor refactor bringing the point marker coercion (from #2607) into the handler instead of requiring callsites to pass a marker type. No user-facing change.
(disclosure: minor LLM assistance used in writing the test boilerplate, which I reviewed in full. All other changes certified organic and tested in person)
No changelog and user manual edits yet, pending the above question
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test)eldev lint) which is based onelisp-lintand includescheckdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.