Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slime-presentations: have different way of inserting the value #734

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deftransform
Copy link

@deftransform deftransform commented Sep 2, 2022

For example:

USER> (list 1 2 3)
{(1 2 3)} ;; presentation, shown between accolades

USER> (length {(1 2 3)})  ;; same presentation: notice there is no quote

With the existing behavior, there is an error:

...
Debugger invoked on a SB-INT:COMPILED-PROGRAM-ERROR in thread 
#<THREAD "main thread" RUNNING {1004F10113}>:
  Execution of a form compiled with errors.
Form:
  (1 2 3)
Compile-time error:
  illegal function call

With the QUOTE strategy of the patch, the value is self-evaluating, and the above example results in a valid expression.

Remarks

I find the QUOTE approach more logical when thinking about presentations as opaque objects, but I understand that this might not be how other people understand them.

My first fix was to locally change slime-reify-old-output to add a quote for all inserted values. But then I encountered warnings when trying to mutate inserted presentation values (using SBCL).

I personally resolved this by setting swank-repl::*listener-eval-function* to a wrapper that always configure SBCL to use the :INTERPRET evaluation-mode, which seems better for the REPL. But it appears that wrapping the quoted form in EVAL also works to silence the warning.

So, this patch introduces a customization for slime-presentations, i.e. a way to choose how they are inserted. I also added the old behavior, where the form is inserted literally for evaluation (not at read-time), but this might not be necessary.

The default value respects the existing behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant