Skip to content

ClojureScript support #14

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

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

ClojureScript support #14

wants to merge 8 commits into from

Conversation

prepor
Copy link

@prepor prepor commented Aug 13, 2017

Hello!

I think it's cool to have full featured parser library for both platforms. Actually, the biggest change is midje removing and code style changes (:use :all is a bad practice and cljs even doesn't support it).

@blancas
Copy link
Owner

blancas commented Aug 16, 2017

Hi @prepor thanks for all the work. I will review the code as soon as I can and will take it from there. What I've browsed looks good.

@blancas
Copy link
Owner

blancas commented Aug 22, 2017

I wanted to try a few things at the REPL but found some things I don't know how handle yet.

$ lein figwheel
Figwheel: Cutting some fruit, just a sec ...
Figwheel: Validating the configuration found in project.clj
Spec Warning:  missing an :output-to option - you probably will want this ...
Figwheel: Configuration Valid ;)
Figwheel: Starting server at http://0.0.0.0:3449
Figwheel: Watching build - dev
Compiling "main.js" from ["src"]...
WARNING: Namespace blancas.kern.char contains a reserved JavaScript keyword, the corresponding Google Closure namespace will be munged to blancas.kern.char$ at line 1 out/blancas/kern/char.cljs
WARNING: Use of undeclared Var blancas.kern.core/slurp at line 75 out/blancas/kern/core.cljc
WARNING: Use of undeclared Var blancas.kern.core/slurp at line 76 out/blancas/kern/core.cljc
WARNING: Bad method signature in protocol implementation, Comparable does not declare method called compareTo at line 110 out/blancas/kern/core.cljc
WARNING: Use of undeclared Var blancas.kern.core/Comparable at line 110 out/blancas/kern/core.cljc
WARNING: Symbol Comparable is not a protocol at line 110 out/blancas/kern/core.cljc
WARNING: Use of undeclared Var blancas.kern.core/printf at line 965 out/blancas/kern/core.cljc
WARNING: Use of undeclared Var blancas.kern.core/parse-file at line 1054 out/blancas/kern/core.cljc
Successfully compiled "main.js" in 8.048 seconds.

This is fine:

(k/run (k/return "foo") "xyz")
"foo"

nil

But this fails:

(k/run (k/fail "this is bad") "xyz")
#object[TypeError TypeError: undefined is not an object (evaluating 'blancas.kern.core.printf.call')]
blancas$kern$core$print_error@file:///Users/arblanca/dev/hacks/cljs/hello_kern/out/blancas/kern/core.js:2702:25
cljs$core$IFn$_invoke$arity$4@file:///Users/arblanca/dev/hacks/cljs/hello_kern/out/blancas/kern/core.js:2745:35
blancas$kern$core$run@file:///Users/arblanca/dev/hacks/cljs/hello_kern/out/blancas/kern/core.js:2723:59
cljs$core$IFn$_invoke$arity$2@file:///Users/arblanca/dev/hacks/cljs/hello_kern/out/blancas/kern/core.js:2733:34
blancas$kern$core$run@file:///Users/arblanca/dev/hacks/cljs/hello_kern/out/blancas/kern/core.js:2715:59


eval code
eval@[native code]
figwheel$client$utils$eval_helper@file:///Users/arblanca/dev/hacks/cljs/hello_kern/out/figwheel/client/utils.js:160:12
nil

This is a good chance for me to learn some basic ClojureScript. Will try to fix those errors.

@prepor
Copy link
Author

prepor commented Aug 22, 2017

I'm sorry, for this far from ideal PR.

I've fixed bunch of issues since the initial commit.

@blancas
Copy link
Owner

blancas commented Aug 23, 2017

Cool. I'm just going to to through the examples in the wiki and the sample programs to make sure things work fine on both platforms.

@blancas
Copy link
Owner

blancas commented Aug 23, 2017

I figured I had to prepare my build with this:

$ npm install -g karma-cli
$ npm install karma-safari-launcher karma-opera-launcher --save-dev
$ npm install karma-cljs-test

$ lein doo safari test
LOG: 'Testing blancas.kern.test-core'
LOG: 'Testing blancas.kern.test-lexer'
Safari 10.1.2 (Mac OS X 10.12.6) blancas.kern.test-lexer test-0200 FAILED
	Fail (test-0200) (cljs/test.js:372:17)
	Expected (= c1 "function Number() { [native code] }")
	Actual: (not (= "function Number() {\n    [native code]\n}" "function Number() { [native code] }"))
	
Safari 10.1.2 (Mac OS X 10.12.6) blancas.kern.test-lexer test-0245 FAILED
	Fail (test-0245) (cljs/test.js:372:17)
	Expected (= c1 "function Number() { [native code] }")
	Actual: (not (= "function Number() {\n    [native code]\n}" "function Number() { [native code] }"))
	
Safari 10.1.2 (Mac OS X 10.12.6) blancas.kern.test-lexer test-0300 FAILED
	Fail (test-0300) (cljs/test.js:372:17)
	Expected (= c1 "function Number() { [native code] }")
	Actual: (not (= "function Number() {\n    [native code]\n}" "function Number() { [native code] }"))
	
LOG: 'Testing blancas.kern.test-lexer-c'
LOG: 'Testing blancas.kern.test-lexer-haskell'
LOG: 'Testing blancas.kern.test-lexer-java'
LOG: 'Testing blancas.kern.test-lexer-shell'
Safari 10.1.2 (Mac OS X 10.12.6): Executed 620 of 620 (3 FAILED) (1.988 secs / 10 mins 1.046 secs)

Looks like we're almost there. In addition, I ran examples from the wiki and it was mostly just fine (primitive and combinators), except for the bind macro. I understand it has to be declared in a previously-compiled file, but I haven't looked too carefully at how macros are declared in ClojureScript.

dev:blancas.kern.core=> (run (bind [d digit l letter] (return [d l])) "1A")
----  Compiler Warning on   <cljs form>   line:1  column:7  ----

  Use of undeclared Var blancas.kern.core/bind

  1  (run (bind [d digit l letter] (return [d l])) "1A")
           ^--- 

I'll continue to look into this as time allows.

@blancas
Copy link
Owner

blancas commented Aug 31, 2017

@prepor The failing tests just need to have a target string with embedded new lines, so comparing againts this string will fix them:

"function Number() {\n    [native code]\n}"

My problem with bind was trying to use that macro within its own namespace. From another it works fine:

dev:cljs.user=> (k/run (k/bind [d k/digit l k/letter] (k/return [d l])) "1A")
nil
dev:cljs.user=> ["1" "A"]

I tried the sample programs and it was mostly fine, except for the i18n part.

Because of the separation between default and default-fmt, the function i18n-merge isn't merging several keys. In addition, the user should only provide key-value pairs. But the way the default-ftm map is coded it would require the user to provide anonymous functions containing the desired text.

It's simpler for the user if the code of this file won't require any changes https://github.com/blancas/kern/blob/master/src/main/resources/local.clj

The result should be:

local=> (run digit "x")
línea 1 columna 1
no se esperaba \x
se esperaba dígito

@prepor
Copy link
Author

prepor commented Sep 4, 2017

Ok. We can use goog.string.format for string formatting. Committed

@rbuchmann
Copy link

Hi, what's the status of this PR? I really would like the cljs support.

@prepor
Copy link
Author

prepor commented Nov 2, 2017

@rbuchmann It works for me and you can try to play with it ([ru.prepor/kern "1.2.0-SNAPSHOT"]). I have implemented the relatively complex parser and there are no known issues.

@rbuchmann
Copy link

@prepor Awesome, I'll try your fork then.

@blancas
Copy link
Owner

blancas commented Nov 2, 2017

@prepor Sorry, dude. I've been in a crunch at work and other things. I'll get back to this when I can.

@jakeisnt
Copy link

jakeisnt commented Feb 6, 2021

Hey, any status on merging this?

1 similar comment
@jakeisnt
Copy link

jakeisnt commented Feb 6, 2021

Hey, any status on merging this?

@blancas
Copy link
Owner

blancas commented Feb 6, 2021

@jakeisnt I got nowhere because I found clojurescript to be rough-going and quite frustrating as dual-language development, the whole js thing, and the tooling; the opposite of my experience coding kern for the fun of it. I recommend you guys fork this and take it from here.

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.

4 participants