Loop reduction over the first 2 elements of the stack #12
Conversation
Before this commit, reading something like "\u \v \w", would produce two char tokens, one with "\u \v " and another with "\w". Test included exemplifies solution.
Now the parser can consume nested tags.
|
Thanks! Left a remark about the test case, would be great if you could add that again. The Hey btw would you be interested to start a post about Unrepl on ClojureVerse? I think it could be an interesting discussion topic. Maybe just a brief introduction about what it is and what the latest developments are... |
|
@plexus I don't see your comments on the code, maybe you need to click the "start review" button or something? I've been thinking a lot about starting a related topic on ClojureVerse, maybe not about Unrepl per se, but about Clojure IDEs in Emacs, things that may be improved from Cider's UI, stuff like that. I just need to organize my thoughts about it a little bit first, and get ready to do a first alpha release of unrepl.el so that people could take a look at it. An Unrepl topic would be great as well, but maybe we can get @cgrand to start it :) |
| @@ -114,15 +114,13 @@ | |||
| (should (equal (parseclj-lex-next) (parseclj-lex-token :character "\\c" 30)))) | |||
|
|
|||
| (with-temp-buffer | |||
| (insert "\\newline\\return\\space\\tab\\a\\b\\c") | |||
There was a problem hiding this comment.
Did you mean to add an extra test case instead of replacing this one? I think it'd be good if we still tested for these special cases like \newline and \return...
There was a problem hiding this comment.
It's just that this test is repeated already: https://github.com/lambdaisland/parseclj/blob/master/test/parseclj-lex-test.el#L105, I separated this change into its own commit so it'd be easier to spot.
parseclj-parser.el
Outdated
| opening-token | ||
| new-stack) | ||
| (setq top-value (parseclj--take-value stack value-p)) | ||
| (setq opening-token (parseclj--take-token (nthcdr (length top-value) stack) value-p '(:discard :tag))) |
There was a problem hiding this comment.
I'd prefer to leave these in the let, that would also make it a bit easier to review. Right now it's hard to see what exactly changed here.
There was a problem hiding this comment.
You're right, I screwed up while reworking the code.
|
Ok, submitted the review now. That UI still confuses me. Don't overthink things, it's not a book or a blog post, it's fine to just share some random notes and thoughts :) |
Leaves `new-stack` as an uninitialized lexical variable so that we compute it only when needed.
Previously the parser would throw on nested tags, complaining for a mismatched
:tag. This commit fixes this behavior by looping through top values and opening tokens.I branched off of
parseclj-lex-character-fixbecause for some reason it got lost in the previous PR merging, so its changes are not in master yet.