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

Adaptations to current Eclector version in software/lisp #11

Closed
wants to merge 2 commits into from

Conversation

scymtym
Copy link

@scymtym scymtym commented Sep 7, 2019

It compiles and seems to work as intended according to visual inspection (using McCLIM's clouseau) of the result of

(parse-asts
 (from-string (make-instance 'lisp)
          "(defun foo () (let ((a 1)) #.(+ 1 2)))"))

Might fix some of the issues in #10.

@eschulte
Copy link
Contributor

Thanks for these changes! I've tested this out (in this new branch scymtym-eclector-update based off your branch) and it looks to be working. I did have to customize eclector.reader:interpret-symbol (in 3fdcd50) so that it wouldn't throw an error when reading from a missing package (we would prefer not to have to load packages to read files). Do you see any problem with this change or suggest a better way?

@scymtym
Copy link
Author

scymtym commented Sep 12, 2019

Do you see any problem with this change or suggest a better way?

That depends on the requirements. As a lightweight solution, a fake package seems appropriate. In other cases such as making a secure reader, not interning at all would be better. For such cases, a model/re-implementation of the package system including symbols might be required. I am using such a solution in an unpublished library and I can say that it adds lots of complications.

So if your solution works for your use-case, I wouldn't change it.

@eschulte
Copy link
Contributor

Thanks for the suggested changes! We'll act on this.

@eschulte eschulte closed this Dec 13, 2019
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.

2 participants