joaotavora / sly-stepper

sly-slepper
27 stars 5 forks source link

Support non-Quicklisp installs #2

Closed Ambrevar closed 3 years ago

Ambrevar commented 3 years ago

There is an issue with the way agnostic-lizard is loaded: it systematically fails if Quicklisp is not installed. This is because of this snippet in slynk-stepper.lisp:

(eval-when (:load-toplevel :execute)
  (format *trace-output* "~&Attempting ~a preload~%" :agnostic-lizard)
  (ignore-errors
   (unless (funcall (read-from-string "ql:quickload")
                    :agnostic-lizard)
     (funcall (read-from-string "asdf:load-system")
              :agnostic-lizard))))

Notice that the unless condition raises a condition when QL is missing.

Easy fix:

(eval-when (:load-toplevel :execute)
  (format *trace-output* "~&Attempting ~a preload~%" :agnostic-lizard)
  (ignore-errors
   (unless (ignore-errors (funcall (read-from-string "ql:quickload")
                                   :agnostic-lizard))
     (funcall (read-from-string "asdf:load-system")
              :agnostic-lizard))))
joaotavora commented 3 years ago

I don't understand: did you add an ignore-errors on the fourth line and that's it? Then how can ignore-errors on that line not catch that raised condition???

Ambrevar commented 3 years ago

Previously, a missing QL would result in asdf:load-system to not be executed. With the ignore-errors, the unless condition becomes nil, so the body of the unless is executed, which works on systems where the :agnostic-lizard package is available in the Common Lisp registry.

joaotavora commented 3 years ago

ah, makes sense. Then I propose it should be simply

(eval-when (:load-toplevel :execute)
  (format *trace-output* "~&Attempting ~a preload~%" :agnostic-lizard)
  (or
   (ignore-errors (funcall (read-from-string "ql:quickload")
                           :agnostic-lizard))
   (funcall (read-from-string "asdf:load-system")
            :agnostic-lizard)))

which makes it error when the ASDF technique also doesn't work. Or maybe we could add another ignore-errors in there to the second clause, and then warn. But I prefer the error.

Maybe we could add a (require :asdf) in there for good measure. Anyway, I don't think multi-level ignore-errors is needed.

Ambrevar commented 3 years ago

Agreed with your propose solution. (I only nested the ignore-errors because it was easier to patch from the Guix package definition.)

joaotavora commented 3 years ago

OK, just pushed the fix. Hopefully you tested it, because I didn't. Don't have time to give much support to this right now, but appreciate you trying this!

Ambrevar commented 3 years ago

Works for the Guix package, thanks!

rongcuid commented 3 months ago

Hello, this issue seems to happen again. I am using Doom Emacs on Mac OS with SBCL. I disabled sly-quicklisp, and got this exact error.