Fix typo #592
Fix typo #592
Conversation
macdavid313
force-pushed
the
master
branch
4 times, most recently
from
925f3da
to
4522453
Compare
especially for Allegro's Modern Mode: https://franz.com/support/tech_corner/modern.mode.lhtml
| @@ -2966,11 +2974,19 @@ soon once non-ASDF loading is removed. (see github#134)") | |||
| Receives a module name as argument and should return non-nil if it | |||
| managed to load it.") | |||
| (:method ((method (eql :slynk-loader)) module) | |||
| (funcall (intern "REQUIRE-MODULE" :slynk-loader) module)) | |||
| (funcall (intern #.(if (eq :UPCASE (readtable-case *readtable*)) | |||
There was a problem hiding this comment.
Here I think we can use uiop:intern*, which can take symbols as arguments, to get a simpler solution: (uiop:intern* '#:require-module '#:slynk-loader)
For that matter, one could even do:
(uiop:symbol-call '#:slynk-loader '#:require-module module)
There was a problem hiding this comment.
Is ASDF/UIOP already a dependency of this file? I don't think so (may be wrong) but I would avoid it. I know Allegro ships with it (in recent versions), but not necessarily all SLY backends do.
| (:method ((method (eql :asdf)) module) | ||
| (unless *asdf-load-in-progress* | ||
| (let ((*asdf-load-in-progress* t)) | ||
| (funcall (intern "LOAD-SYSTEM" :asdf) module))))) | ||
| (funcall (intern #.(if (eq :UPCASE (readtable-case *readtable*)) |
There was a problem hiding this comment.
uiop:symbol-call here, too.
| @@ -2979,9 +2995,15 @@ managed to load it.") | |||
| (:documentation | |||
| "Using METHOD, consider PATH when searching for modules.") | |||
| (:method ((method (eql :slynk-loader)) path) | |||
| (add-to-load-path-1 path (intern "*LOAD-PATH*" :slynk-loader))) | |||
| (add-to-load-path-1 path (intern #.(if (eq :UPCASE (readtable-case *readtable*)) | |||
There was a problem hiding this comment.
(uiop:intern* '#:*load-path* '#:slynk-loader)
| (:method ((method (eql :asdf)) path) | ||
| (add-to-load-path-1 path (intern "*CENTRAL-REGISTRY*" :asdf)))) | ||
| (add-to-load-path-1 path (intern #.(if (eq :UPCASE (readtable-case *readtable*)) |
| @@ -3219,7 +3241,9 @@ QUALIFIERS and SPECIALIZERS are lists of strings." | |||
| (mapcar (lambda (specializer) | |||
| (if (typep specializer 'slynk-mop:eql-specializer) | |||
| (format nil "(eql ~A)" | |||
| (slynk-mop:eql-specializer-object specializer)) | |||
| (funcall #+allegro 'mop:eql-specializer-object | |||
There was a problem hiding this comment.
Is it intended that this will only work on Allegro or SBCL? I don't know enough about the context.
|
To help in reviewing this, is this PR still actual? What problem is it solving? Is it really just "fixing a typo" as the title suggests? |
| @@ -1487,9 +1487,11 @@ Return :interrupt if an interrupt occurs while waiting." | |||
| (error | |||
| "~s not implemented. Check if ~s = ~s is supported by the implementation." | |||
| 'wait-for-input | |||
| (slynk-backend:find-symbol2 "SLYNK:*COMMUNICATION-STYLE*") | |||
| (slynk-backend:find-symbol2 #1=#.(if (eq :UPCASE (readtable-case *readtable*)) | |||
There was a problem hiding this comment.
Can't we hide whatever logic we need to hide inside SLYNK-BACKEND:FIND-SYMBOL2?
|
I think what may be happening here is that @macdavid313 opened a PR from his fork's Anyway, it seems like at least some of those are fixing actual problems for Allegro "modern mode", so those fixes would be welcome, maybe as separate well-contextualized pull requests |
|
@joaotavora Apologies for such a long-due, careless, and dangling PR. It's not intended. I think, at the beginning, this PR used to be rather small; after a while, I lost the track of it and force-pushed my new updates to it. I will make sure the next update comes together with a "Ready for review" status. @rpgoldman thanks for your suggestions! Appreciate it. |
find-symbo2l -> find-symbol2