Re: ClojureScript source maps

[David Nolen]

On Fri, Mar 23, 2012 at 10:51 PM, Brandon Bloom <snpr...@gmail.com> wrote:

> The first step would be just to get the compiler to emit an optional source map.

That's definitely the first step! After that, there's going to need to be some way to serve those source maps via HTTP upon request from the browser.

However, there's a first step to the first step: Getting column data. The reader currently seems to only produce a :line key in the metadata map.

Yep. This would require enhancing the Clojure reader. I don't think that's feasible for 1.4, but probably worth pursuing for 1.5. Even without it, we can still get line level debugging which is better than nothing and write the code to use :column data when available.

 

Could you please clarify? Do you mean for multi-level mapping?

Yes. Again this is really just bonus points. Priority 1 is just improving the development experience.

 

See the "Multi-level Mapping Notes" section in the v3 spec:

https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit?hl=en_US&pli=1&pli=1

It seems like once they agree upon a header (as opposed to the javascript comment syntax) this would also be useful for providing pre-macro-expansion vs post-macro-expansion source maps.

 We could also decide to handle multi-mapping ourselves - that is write the code that can produce a merged source map.

David

[Brandon Bloom]

 We could also decide to handle multi-mapping ourselves - that is write the code that can produce a merged source map.

It seems likely (already done?) for browsers to support the X-SourceMap header and //@ comments for the target use case of CoffeeScript -> Javascript -> Minified Javascript

Also, it seems preferable to let the browser do it, so their debugger can operate at various levels of source translation depending on your needs.

So why bother doing it ourselves? For the Rhino environment?

[David Nolen]

Or SpiderMonkey or V8 or Node.js. Or whatever fast JS runtimes may appear in the future :)

David

[Brandon Bloom]

OK makes sense.

I've been studying the compiler and trying to figure out an approach to implementing source maps.

The main issue seems to be keeping track of line numbers in the generated file. The use of (print (str ...)), emits, and with-out-str makes it tricky, unless you replace all print calls with something that counts newlines.

It's pretty straightforward to replace println with a line counting helper -- let's call it emitln -- and break up strings at \n with new calls to emitln. But this still doesn't address the various emits situations which internally print their own new lines.

The most robust, but most invasive change would be to rework the emit multimethod to return a sequence of lines instead of printing as a side effect. I've got some ideas for doing that with minimal pain, but it's still gonna be a lot of churn. At minimum, all calls to print and println will need to be replaced with a line-tracking helper.

Any other ideas?

[Brandon Bloom]

Actually, that wasn't too bad.

I'm tackling source maps on this branch here: https://github.com/brandonbloom/clojurescript/tree/source-maps

At the time of this posting (e8cf369648), I've reworked the compiler to avoid the emits, with-out-str, etc. The result is that I am able to generate an otherwise blank source map file with the correct number of semicolons (that is, one semicolon for each generated line of code).

I'm going to forge ahead and see if I can get some symbols mapped & get something working in the Chrome Canary environment.

[Brandon Bloom]

I'm going to forge ahead and see if I can get some symbols mapped & get something working in the Chrome Canary environment.

It looks like this could take quite some time :-)

In the meantime, I've cleaned up a branch which implements what I think is a necessary first step for anyone who wants to implement source-maps: Tracking the line and column of the generated javascript.

See this branch here: https://github.com/brandonbloom/clojurescript/compare/clojure:master...brandonbloom:track-pos

It's basically two commits. The first replaces all calls to print, println, emits, etc, such that all code generation funnels through the new emitx and emitln. The second commit modifies emitx and emitln to track *position* as a 2-tuple vector of [line, column], zero-based. The line and column information is a prerequisite for generating correct source maps.

David (and other core contributors): Does this seem like a sane approach? I've got decent confidence in the patches' quality, as I was diffing the compiled output of the twitterbuzz sample as I worked. Is it worth scrutinizing and merging this patch before further development on source-maps?

I don't know how the cljs core team likes to work, but I prefer to make small, targeted changes and get them into main line, rather than suffer through merging long-lived branches. Especially considering I might not get a free day to dedicate to Clojure again anytime soon :-)

[David Nolen]

On Mon, Mar 26, 2012 at 1:16 AM, Brandon Bloom <snpr...@gmail.com> wrote:

I'm going to forge ahead and see if I can get some symbols mapped & get something working in the Chrome Canary environment.

It looks like this could take quite some time :-)

I'm assuming you're referring to the source map generation part?

 

In the meantime, I've cleaned up a branch which implements what I think is a necessary first step for anyone who wants to implement source-maps: Tracking the line and column of the generated javascript.

See this branch here: https://github.com/brandonbloom/clojurescript/compare/clojure:master...brandonbloom:track-pos

It's basically two commits. The first replaces all calls to print, println, emits, etc, such that all code generation funnels through the new emitx and emitln. The second commit modifies emitx and emitln to track *position* as a 2-tuple vector of [line, column], zero-based. The line and column information is a prerequisite for generating correct source maps.

David (and other core contributors): Does this seem like a sane approach? I've got decent confidence in the patches' quality, as I was diffing the compiled output of the twitterbuzz sample as I worked. Is it worth scrutinizing and merging this patch before further development on source-maps?

The approach sounds sensible! I haven't had time to look at the patch in detail - but will do so and  encourage anyone else to do so as well.

 

I don't know how the cljs core team likes to work, but I prefer to make small, targeted changes and get them into main line, rather than suffer through merging long-lived branches. Especially considering I might not get a free day to dedicate to Clojure again anytime soon :-)

Again, sounds like a good starting point - I've made a new track-pos branch on the main repo with your changes. Would love to have somebody take it the rest of the way.

David

[Brandon Bloom]

It looks like this could take quite some time :-)

I'm assuming you're referring to the source map generation part?

Yup.

 

Again, sounds like a good starting point - I've made a new track-pos branch on the main repo with your changes. Would love to have somebody take it the rest of the way.

Great. If I get any spare time (maybe next weekend?) I'll take a look at getting the corresponding column numbers from the Clojure reader. I figure if it's way too late for 1.4, that change should get made early, so it's got a fighting chance for 1.5 :-)

[Brandon Bloom]

Great. If I get any spare time (maybe next weekend?) I'll take a look at getting the corresponding column numbers from the Clojure reader. I figure if it's way too late for 1.4, that change should get made early, so it's got a fighting chance for 1.5 :-)

Ticket with work in progress patch for column metadata from the Clojure reader: http://dev.clojure.org/jira/browse/CLJ-960 

[Brandon Bloom]

David: I'm pretty new to the community, so I'd appreciate some help navigating the processes and norms.

What should I do to help move the source maps forward? What should I expect in terms of getting my two patches vetted, improved as necessary, and applied?

I'd like to contribute more time to this project, but I want to make sure I'm not just spinning my wheels.

[David Nolen]

On Fri, Mar 30, 2012 at 4:04 PM, Brandon Bloom <snpr...@gmail.com> wrote:

David: I'm pretty new to the community, so I'd appreciate some help navigating the processes and norms.

What should I do to help move the source maps forward? What should I expect in terms of getting my two patches vetted, improved as necessary, and applied?

The Clojure column patch will need to wait for post 1.4.0 release.

 

I'd like to contribute more time to this project, but I want to make sure I'm not just spinning my wheels.

I've been a bit busy with CLJS @ JSConf things so I haven't had time to look over at your CLJS compiler patch. If somebody doesn't get to it first with notes I'll probably have some time later next week.

David

[David Nolen]

I applied your other minor patches.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To view this discussion on the web visit https://groups.google.com/d/msg/clojure-dev/-/Bk4ogHDfVD4J.

To post to this group, send email to cloju...@googlegroups.com.
To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.

[Andy Fingerhut]

This is no guarantee of quickness of accepting your changes into Clojure/JVM, but if you create a git format patch of your changes and attach it to the JIRA ticket [1], my prescreening automation process will pick it up and check it for some easily-automated things (e.g. applies cleanly, builds, passes tests), and go on my what-might-become-weekly updated list of prescreened patches.

[1] If you've got a git repo with your changes, this is pretty easy. Instructions here:

http://dev.clojure.org/display/design/JIRA+workflow

Look under the "Development" heading.

Andy

[Brandon Bloom]

I applied your other minor patches.

Great, thanks!

The Clojure column patch will need to wait for post 1.4.0 release.

OK, I'll poke my head up again after that happens.

 

I've been a bit busy with CLJS @ JSConf things so I haven't had time to look over at your CLJS compiler patch. If somebody doesn't get to it first with notes I'll probably have some time later next week.

Understood. Thanks. 

[Brandon Bloom]

This is no guarantee of quickness of accepting your changes into Clojure/JVM, but if you create a git format patch of your changes and attach it to the JIRA ticket [1], my prescreening automation process will pick it up and check it for some easily-automated things (e.g. applies cleanly, builds, passes tests), and go on my what-might-become-weekly updated list of prescreened patches.

Gotcha. Wasn't looking for a concrete timeline, just wanted to set my own expectations. Sometimes there is politics and whatnot to navigate and I simply don't know the right people to talk yet.

Specifically regarding my column numbers work, I'll wait until after 1.4 is released, rebase my changes up to the latest, scrutinize the patch more heavily myself, and then make sure I show up on your radar then. Thanks!

 

[1] If you've got a git repo with your changes, this is pretty easy.

I've following that process with my prior patches that David just applied. I really wish pull requests were accepted though, as JIRA is cumbersome and unfamiliar.

[David Nolen]

Brandon, 

Can you please generate a patch with your changes and create the corresponding ticket in JIRA? I took a look at the patch - it looks mostly good, let's take the conversation over there.

Also you should ping Clojure/core so that you can own tickets in JIRA.

Thanks!

David

--

You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To view this discussion on the web visit https://groups.google.com/d/msg/clojure-dev/-/z9My_wIIgWkJ.

[Brandon Bloom]

Sorry for the delay. There were some merge conflicts & I wanted to ensure a good patch.

Moving the conversation to this ticket: http://dev.clojure.org/jira/browse/CLJS-176

Thanks!

David

To unsubscribe from this group, send email to clojure-dev+unsubscribe@googlegroups.com.