-
Notifications
You must be signed in to change notification settings - Fork 329
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
[wanted] make the error condition also a presentation, just like normal results #28
Comments
I'd like to "vote up" this feature request. I believe the issue is even worse than the original bug report. Not only isn't the error a presentation, the error report is not present in the SLIME REPL buffer, either. That means that when you quit the debugger, you have even lost the error message. |
@rpgoldman, @attila-lendvai: got any implementation suggestions? |
I'm afraid I'm not an expert on SLIME internals. But if someone would point me at where SLIME would encounter an error, I'd be willing to try to submit a patch. I see SLIME-DISPATCH-EVENT, which seems to be what triggers SLDB, but I'm not sure how that gets invoked, and how the code in slime.el and contrib/slime-repl.el interact. |
@rpgoldman I'm not an expert either. The juicy bits on the CL side are in SLDB-LOOP. I suppose the first challenge is to detect, in that context, that the restart set up by WITH-TOP-LEVEL-RESTART was invoked. |
I think we have to get this BEFORE we get into SLDB. We should be presenting the |
Ah, I was assuming we would only show the condition when the evaluation is aborted. Currently, the textual description of the condition is only printed to the REPL in that scenario. I interpreted this feature request as "simply" changing that textual description into a presentation. |
The textual description is printed, but we don't print the error message (the result of invoking the |
Printing the error message is not quite the same as outputting a presentation. It's probably useful regardless. :) |
i think the main issue here is that the presentation infrastructure was pushed out into a contrib, and as such sldb cannot depend on that in the current status quo (unless we make sldb also a contrib (yes, that's a joke)). |
With respect to luismbo's comment, I think we want both: (1) we want to output the condition object as a presentation that we can inspect, etc. (2) we should output the error message into the slime repl buffer before we pop up SLDB (in particular so that we can still see the error message after the SLDB buffer has gone away). Actually, AFAICT outputting the presentation object into the buffer should also be done before popping up the SLDB buffer. |
attila-lendvai -- is there a way to check whether the presentation infrastructure is available, and based on that either write the condition as a presentation or simply as a string? |
@attila-lendvai I don't think that's a major barrier. We can always add a hook that the presentations contrib can take advantage of. @rpgoldman I don't think (2) implies printing before SLDB pops up. The current message Sorry I've only had a chance to poke at this in very short bursts. Here's my first, obviously flawed attempt. (Didn't update the protocol everywhere so this will yield some protocol errors here and there.) diff --git a/swank.lisp b/swank.lisp
index 638d4f6..5ca49ce 100644
--- a/swank.lisp
+++ b/swank.lisp
@@ -1746,7 +1746,8 @@ Errors are trapped and invoke our debugger."
(send-to-emacs `(:return ,(current-thread)
,(if ok
`(:ok ,result)
- `(:abort ,(prin1-to-string condition)))
+ `(:abort ,(prin1-to-string condition)
+ ,(princ-to-string condition)))
,id)))))
(defvar *echo-area-prefix* "=> "
diff --git a/contrib/slime-repl.el b/contrib/slime-repl.el
index 2158632..c4d926f 100644
--- a/contrib/slime-repl.el
+++ b/contrib/slime-repl.el
@@ -546,8 +546,8 @@ joined together."))
(slime-lisp-package))
((:ok result)
(slime-repl-insert-result result))
- ((:abort condition)
- (slime-repl-show-abort condition))))
+ ((:abort condition message)
+ (slime-repl-show-abort condition message))))
(defun slime-repl-insert-result (result)
(with-current-buffer (slime-output-buffer)
@@ -563,7 +563,7 @@ joined together."))
(slime-repl-insert-prompt))
(slime-repl-show-maximum-output)))
-(defun slime-repl-show-abort (condition)
+(defun slime-repl-show-abort (condition message)
(with-current-buffer (slime-output-buffer)
(save-excursion
(slime-save-marker slime-output-start
@@ -571,6 +571,9 @@ joined together."))
(goto-char slime-output-end)
(insert-before-markers (format "; Evaluation aborted on %s.\n"
condition))
+ (insert-before-markers (mapconcat (lambda (line)
+ (format "; %s\n" line))
+ (split-string message "[\r\n]") ""))
(slime-repl-insert-prompt))))
(slime-repl-show-maximum-output))) |
Is there some "right way" to work on this with you? E.g., make a topic branch for this feature on github so that we could work on it together? If it's possible to help.... |
@rpgoldman Pushed that patch here: https://github.com/luismbo/slime/tree/issue-28 I guess the git way to do it is that you fetch from there and then push changes to a fork of yours from which I can fetch and so on. |
Youtube plugin cleanup
bug imported from https://bugs.launchpad.net/slime/+bug/806991
when aborting an eval in the repl due to an error, then it would be quite useful if the error condition was also a presentation.
e.g. sbcl's nested error guard aborts with the error, the debugger is not shown, and only a print1-to-string is shown of the error condition in slime.
The text was updated successfully, but these errors were encountered: