ClojureScript

Pass along compiler options added to REPL environments

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

Many of the compiler options (esp. :libs, :externs, etc.) that are accepted by the compiler (and commonly specified via lein-cljsbuild options) should also apply to ClojureScript REPL sessions. Since all known IJavaScriptEnv implementations are records, the most straightforward way to do this is to allow one to add such options to REPL environment records, which can then be passed on to the compiler in cljs.repl.

Making this change has had a big impact on my personal ClojureScript productivity. Interested in the patch?

Activity

Hide
David Nolen added a comment -

Patch welcome!

Show
David Nolen added a comment - Patch welcome!
Hide
Chas Emerick added a comment -

Awesome; will put one up once the disposition of CLJS-489 is settled, since the browser-repl impl details are obviously implicated somewhat.

Show
Chas Emerick added a comment - Awesome; will put one up once the disposition of CLJS-489 is settled, since the browser-repl impl details are obviously implicated somewhat.
Hide
Chas Emerick added a comment - - edited

AFAICT, the only options where this makes sense are :libs and :foreign-libs. These two are needed to allow you to have dependencies loaded as they would be using e.g. lein-cljsbuild. The rest of the ClojureScript compiler options are of relatively minimal use in the REPL.

This conveniently keeps the scope of the change down, affecting only the dependency-loading bits in cljs.repl.

I'll update the relevant wiki page to reflect this change once this is applied.

Show
Chas Emerick added a comment - - edited AFAICT, the only options where this makes sense are :libs and :foreign-libs. These two are needed to allow you to have dependencies loaded as they would be using e.g. lein-cljsbuild. The rest of the ClojureScript compiler options are of relatively minimal use in the REPL. This conveniently keeps the scope of the change down, affecting only the dependency-loading bits in cljs.repl. I'll update the relevant wiki page to reflect this change once this is applied.
Hide
Chas Emerick added a comment -

FYI, the attached patch applies cleanly on master after the latest release and still works as expected.

Show
Chas Emerick added a comment - FYI, the attached patch applies cleanly on master after the latest release and still works as expected.
Hide
David Nolen added a comment -

I don't understand why you are using :simple optimizations here. Can you explain?

Show
David Nolen added a comment - I don't understand why you are using :simple optimizations here. Can you explain?
Hide
Chas Emerick added a comment -

That's only the default used if the REPL environment doesn't specify one, and if the REPL environment uses Closure optimization to begin with. The only stock REPL environment that does any optimization is the browser REPL, which defaults to :simple; without anything else to go on, it seemed reasonable to copy it.

The only alternative would be :whitespace. I have no objection to defaulting to that, but I don't see any practical difference between the two given the REPL context.

Show
Chas Emerick added a comment - That's only the default used if the REPL environment doesn't specify one, and if the REPL environment uses Closure optimization to begin with. The only stock REPL environment that does any optimization is the browser REPL, which defaults to :simple; without anything else to go on, it seemed reasonable to copy it. The only alternative would be :whitespace. I have no objection to defaulting to that, but I don't see any practical difference between the two given the REPL context.
Hide
David Nolen added a comment -

Yes but I don't understand why :none is not applicable here - any explanation?

Show
David Nolen added a comment - Yes but I don't understand why :none is not applicable here - any explanation?
Hide
Chas Emerick added a comment -

OK, I had no idea :none was even an option; found where it comes into play just now when I grepped cljs.closure. (I saw the case in cljs.closure/make-options, and worked from there.) I'll redo the patch with :none, test with some downstream REPL envs to make sure everything works as expected, and re-attach.

Show
Chas Emerick added a comment - OK, I had no idea :none was even an option; found where it comes into play just now when I grepped cljs.closure. (I saw the case in cljs.closure/make-options, and worked from there.) I'll redo the patch with :none, test with some downstream REPL envs to make sure everything works as expected, and re-attach.
Hide
Chas Emerick added a comment -

Updated patch attached that defaults :optimizations to :none. Works fine in tests with downstream REPL envs, etc.

Show
Chas Emerick added a comment - Updated patch attached that defaults :optimizations to :none. Works fine in tests with downstream REPL envs, etc.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: