-
Notifications
You must be signed in to change notification settings - Fork 4
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
[WIP] Better quotations #349
base: master
Are you sure you want to change the base?
Conversation
- Extend custom `MetaParser` to allow syntax-changes in the quasiquote - Add helper class for detecting dotted arguments e.g. `[..$xs]` - Add support for splicing multiple `LambdaTerms` into lists Todo: - Other possible ways to splice terms into other terms - Unlifting and unsplicing `LambdaTerms`
I created too huge patterns to be matched against. Those crashed the compiler. This change optimizes the pattern (and the runtime const) of huge constants (numbers and characters) and other terms, whose representation is very large (int-lists and char-lists/strings).
a60604b
to
ebc3942
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QuotationMacro.scala
is the bigest source code file in the entire codebase
$ ls -lhS $(find -type f -name \*.scala)
-rw-r--r-- 1 rexim users 13K Apr 23 02:40 ./macros/src/main/scala/me/rexim/morganey/meta/QuotationMacro.scala
-rw-r--r-- 1 rexim users 9.5K Feb 16 15:37 ./kernel/src/test/scala/me/rexim/morganey/ParserSpec.scala
-rw-r--r-- 1 rexim users 7.0K Apr 23 02:40 ./macros/src/test/scala/me/rexim/morganey/meta/InterpolatorSpec.scala
-rw-r--r-- 1 rexim users 4.6K Feb 16 15:37 ./kernel/src/main/scala/me/rexim/morganey/syntax/LambdaParser.scala
-rw-r--r-- 1 rexim users 4.6K Feb 16 15:37 ./kernel/src/main/scala/me/rexim/morganey/ast/LambdaTerm.scala
...
I cannot read it. Please decompose it accordingly.
case None => out = None | ||
} | ||
out.map(_.result()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it demands some additional tests. I understand the intention here to make it generic, but the implementation is too hard for me 😿
I agree. That's due to new big methods that contain many nested functions. Probably we should somehow extract their contents to a new module with well-defined semantic. |
Thank you guys for your review!
I'll look into that!
There are a few tests for the previous, non-generic Unfortunately, I'm very busy at the moment, so I'll look into it, when I have some time to spare. Apologies in advance for the inconvenience of leaving this PR untouched for a while. |
@keddelzz yeah, I understand. I'm busy too. :) That's why I answer with such a delay. Don't worry, take your time. |
see #345