add source maps support, fixes #50

add source maps support, fixes #50

R=sigmund@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/4c600390f363826b24c66e83301dc1353034991e

[Jennifer Messerly, 2015-02-27 22:21:02]

Patchset #1 (id:1) has been deleted

[Jennifer Messerly, 2015-02-27 22:22:10]

jmesserly@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2015-02-27 22:47:10]

lgtm, just minor comments below

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1724: var outDir = path.dirname(outputPath);
I'm inclined to just inline this in the call two lines below to avoid confusion
with the outer `outDir` inherited from the superclass.

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1747: String jsToString(JS.Node node) {
is this used anywhere? (I couldn't find it)

[Jennifer Messerly, 2015-02-27 22:49:03]

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1724: var outDir = path.dirname(outputPath);
On 2015/02/27 22:47:09, Siggi Cherem (dart-lang) wrote:
> I'm inclined to just inline this in the call two lines below to avoid
confusion
> with the outer `outDir` inherited from the superclass.

sounds good

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1747: String jsToString(JS.Node node) {
On 2015/02/27 22:47:09, Siggi Cherem (dart-lang) wrote:
> is this used anywhere? (I couldn't find it)

no, but I keep adding it back for debugging and it's painful to write it over
and over :)

[Jennifer Messerly, 2015-02-27 22:56:10]

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/965033002/diff/40001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1747: String jsToString(JS.Node node) {
On 2015/02/27 22:49:02, John Messerly wrote:
> On 2015/02/27 22:47:09, Siggi Cherem (dart-lang) wrote:
> > is this used anywhere? (I couldn't find it)
> 
> no, but I keep adding it back for debugging and it's painful to write it over
> and over :)

I could add this in a comment. It could also be added to JS.Node.toString(),
similar to what analyzer does? Maybe the fear was accidentally calling toString,
but it doesn't seem to be a problem, at least with analyzer trees. In that case
it could be called toSource().

[Jennifer Messerly, 2015-02-27 23:17:17]

Tried to address these. Landing but let me know thoughts on
`debugJsNodeToString`

[Jennifer Messerly, 2015-02-27 23:17:24]

Committed patchset #3 (id:60001) manually as
4c600390f363826b24c66e83301dc1353034991e (presubmit successful).

[Siggi Cherem (dart-lang), 2015-02-27 23:26:23]

still lgtm :)

https://codereview.chromium.org/965033002/diff/60001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/965033002/diff/60001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1795: printer.mark(new SourceMapSpan(loc, end,
name, isIdentifier: true));
btw - I just realized this, in reporter.dart we do compute these using
SourceFile... we could switch there to do a similar trick as you do here, or if
possible, put the logic in a shared place (utils.dart?)

[Jennifer Messerly, 2015-02-27 23:30:45]

https://codereview.chromium.org/965033002/diff/60001/lib/src/codegen/js_codeg...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/965033002/diff/60001/lib/src/codegen/js_codeg...
lib/src/codegen/js_codegen.dart:1795: printer.mark(new SourceMapSpan(loc, end,
name, isIdentifier: true));
On 2015/02/27 23:26:23, Siggi Cherem (dart-lang) wrote:
> btw - I just realized this, in reporter.dart we do compute these using
> SourceFile... we could switch there to do a similar trick as you do here, or
if
> possible, put the logic in a shared place (utils.dart?)

yes probably :)
opened https://github.com/dart-lang/dev_compiler/issues/73