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

ClojureScript merge #38

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

ClojureScript merge #38

wants to merge 14 commits into from

Conversation

quoll
Copy link

@quoll quoll commented Jan 10, 2020

I know that you're not a fan of .cljc files, but there are many benefits, and so I'm proposing these changes.

By having the same code for both runtimes, future changes only need to be made in one place, and testing is consistent between both platforms. This also meant that I removed the ClojureScript-only test. It was unnecessarily dependent on being run in a web page (which is awkward to automate). It's not possible to run the ClojureScript tests with:

$ lein cljsbuild test

I've tried to minimize the use of #?(...) macros. The primary difference is in the namespace declarations, due to the differences in referring to macros and importing.

There were also some bugs in the test code. The bf parser did parse correctly (resulting in expected errors or not), but did not return anything. The bencode parser used read-string which is not available in ClojureScript.

Also, I added the position structure to the error continuation object. It's not really needed, but because I moved to using ex-info then the ExceptionInfo structure was asking for some info, and this seemed appropriate.

@quoll
Copy link
Author

quoll commented Jan 13, 2020

Ah, I missed that PR #37 was doing this as well.

Unfortunately, I pushed to the branch that this PR was based on with a completely separate change that I am hoping for: renaming of char/char? to ch/ch?.

The reason for this change is because it's an annoying pain to have to use refer-clojure/excuse for these functions. It wasn't so bad doing it in Clojure, because the effect of it was restricted to the namespace that needed it (specifically, a language defining file), but ClojureScript can import large blocks of code without namespaces, and it can result in warnings if you require a namespace that in turn requires parsatron.

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.

1 participant