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

Bug: Reader::CommonLisp doesn't work as documented #21

Closed
marnen opened this issue Oct 8, 2022 · 5 comments · Fixed by #22
Closed

Bug: Reader::CommonLisp doesn't work as documented #21

marnen opened this issue Oct 8, 2022 · 5 comments · Fixed by #22
Assignees
Labels
bug Something isn't working

Comments

@marnen
Copy link

marnen commented Oct 8, 2022

Describe the bug
The README claims that Reader::CommonLisp treats special atoms specially:

SXP::Reader::CommonLisp.read %q((or t nil))          #=> [:or, true, nil]

However, in practice, Reader::CommonLisp doesn't map Lisp t and nil to Ruby true and false; it just treats them like any other atoms and converts them to Ruby symbols:

irb(main):008:0> SXP::VERSION.to_s
=> "1.2.2"
irb(main):009:0> SXP::Reader::CommonLisp.read %q((or t nil)) # the same example from the README
=> [:or, :t, :nil]
irb(main):010:0> RUBY_VERSION
=> "2.7.1"

I think this is a bug. Either the documentation or the gem should be updated.

@gkellogg
Copy link
Collaborator

gkellogg commented Oct 8, 2022

@marnen, see if PR #22 solves the issue. It adds case-insensitive atoms 't' and 'nil'. I didn't see any other such tokens defined in CommonLisp beyond those defined in the Basic reader.

@marnen
Copy link
Author

marnen commented Oct 8, 2022

Thanks, I’ll look. I think it’s probably more sensible to have Lisp nil map to Ruby nil than to false

@gkellogg
Copy link
Collaborator

gkellogg commented Oct 8, 2022

Thanks, I’ll look. I think it’s probably more sensible to have Lisp nil map to Ruby nil than to false

The are both Falsey, so that would be fine.

@marnen
Copy link
Author

marnen commented Oct 9, 2022

My thoughts exactly. The falsiness of Ruby nil means that the Common Lisp ambiguity between nil and false is OK, if not great.

@marnen
Copy link
Author

marnen commented Oct 17, 2022

Thank you for fixing this! For future reference, though, this was probably a breaking change, so I'm not sure it should have been a patch release (although it was a bugfix).

BTW, I appreciate this gem so much. I'm using this on a project that I'm translating from Lisp to Ruby, so it's coming in handy for constructing Lisp expressions and parsing Lisp output in my RSpec tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants