Clojurians Log v2

I have the following, but I see :

(defn ^:export -main [& args]
  (.log js/console cljs.core/*command-line-args*))

I read that command-line-args is bound only in repl is that true? I know I can use process.argv but I was wondering if I need to open an issue

process.argv is kinda meh in lumo because it includes lumo itself. (iirc)

yeah I have just tried and you are exactly right

Argv will also include "node" in Node.js

There might be a but wrt command line args

Idk

process.argv includes two items that are not the arguments by default

https://github.com/anmonteiro/lumo/blob/master/src/js/cljs.js#L256

There's a bug right here

but @dominicm is right, if I use it in lumo it will include lumo args as well

here you go

I was grepping for command-line-args and could not understand why I could not find it 😄

We switched yo cljs.core/*command-line-args*

To*

yes, I will push a PR, thanks!

Thank you

I was also looking at the other PR, I will need to ask you something as well, in a bit 😉

Sure

@anmonteiro when you have time, I have an error here a bit difficult to understand for me https://github.com/anmonteiro/lumo/pull/238

@richiardiandrea so the reason you’re getting an error is because you never initialize the CLJS engine

oh yeah, I was trying the suggestion of the comments and it still did not work, it is probably the init issue then

@richiardiandrea that’s also not the place where you wanna add the code

because we check for repl below

yep good we are discussing this, I was thinking about it...I guess this feature is valid also for non-repl usage

actually especially for non-repl usage

@richiardiandrea this is only for non-REPL usage

yep, so we don't want to put it in if (repl)...where do you suggest we put it...I put it there because it is alternative to -m

I didn’t suggest to put it in that if statement

I suggested to place it in the else condition of that if statement

but maybe it should be also checking if (!repl)

yeah ok

we’ll be there if !mainNsName && !repl

same thought

@richiardiandrea does *command-line-args* work with your fix?

yes

tnx for the suggestion 😉

I think we can now add tests for that too

since we run tests in Lumo

successfully passing (cli-options (clj->js *command-line-args*)) and posix-getopts is happy

@richiardiandrea https://github.com/anmonteiro/lumo/blob/master/test/lumo/test_runner.cljs

the only thing I need is to set optind to 0

^ this namespace is the entry point for Lumo tests

checking

so we can add Lumo-specific unit tests and require them from there

or even add unit tests in the other files and run them if test-util/lumo-env?

oh I contributed to that already ah ah 😄 will do my best

thanks!

@richiardiandrea for your current use case it would be just a matter of adding cli opts here https://github.com/anmonteiro/lumo/blob/master/circle.yml#L54

or even abstract that away in a common script that you call from the other ci scripts

I guess that would be more of an integration test (that should be done), then I was thinking of setting *command-line-args* in a fixture or something for unit testing

ah! the other patch does not work either...I am investigating why:

~/git/lumo/build/lumo scripts/lumo/build.cljs 
             ⬆
WARNING: No such namespace: lumo.repl, could not locate lumo/repl.cljs, lumo/repl.cljc, or JavaScript source providing "lumo.repl" at line 1 
             ⬆
WARNING: Use of undeclared Var lumo.repl/run-main-cli-fn at line 1 
command line arguments are required.

k solved and pushed the changes

about the cli args, I think the only useful test is the one you are hinting at, an integration test from command line invocation...I don't see the point in testing something else

agreed

we can punt on that for now too

I’ve been meaning to set up integration tests but haven’t found time to do it yet

@richiardiandrea if that’s something you’d like to contribute, I’m open to ideas

definitely something interesting to think about

there are some linting issues in your PR that’s why CI is failing

@richiardiandrea just running yarn prettier should fix them

or npm run prettier

k

lol why is it adding a comma before a closing paren?

I thought it was a typo and removed it

in ES6 trailing commas before closing parens are valid

(which is how you tell that TC-39 is really focusing on important issues)

🙃

I mean, that it wasn't confuses me

Death by semicolons I guess

lisp is so freeing

@anmonteiro @richiardiandrea I just started trying some basic scripts with Lumo - am I understanding that command-line-args is currently not functioning, but PR 237 will fix that?

@michaelwfogleman that's exactly right

OK, thought I was doing something dumb 😛

if tests are going through and Antonio has time for that, a 1.7.1 might be good. Or master.

@michaelwfogleman once I merge that PR you can brew install --HEAD lumo if you’re on a mac

this is a bad enough bug that warrants cutting a 1.7.1 release

@richiardiandrea just wrap that entire deftest in an if or something

done that, waiting for the results

thanks

does that work locally?

@anmonteiro yes it works

awesome all green :partywombat:

@richiardiandrea tests are failing

Added things to both Travis and AppVeyor confs...did not know you had three...that's good, this should now complete fine

ah cool, missed that