Add tool for viewing source maps.

Add tool for viewing source maps.

R=johnniwinther@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36215

[Johnni Winther, 2014-05-14 12:00:38]

LGTM

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/README.TXT (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/README.TXT:1: This program serves a visualization
of a JavaScript source map file generated by dart2js or pub.
Long line.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:4: import 'dart:io';
Add blank line before import. Maybe add library tag.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:109: command = 'start';
Change 'start' to 'cmd.exe /C start'.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/web/display.dart (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:181:
generatedOutput.children.add(new BRElement());
Replace `new BRElement()` by `new
DivElement()..children.add(getLineNumberElement(lineNumber))`.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:219: div..appendText("
")..children.add(createSpan(token, previous, lineEntry));
Remove `..appendText(" ")`.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:235: if (entry != null)
addTargetLine(line, target[line], entry); else
Add {} to the branches.

[ricow1, 2014-05-14 12:28:08]

dbc

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:32: return;
I would set the exitCode to non zero here - it is good style to give non zero
exit code in case of failure (same above)

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:51: print("Error: $e");
I would indent this 2

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:71: int port = 9223;
I would use an ephemeral port here, you can easily pass that to the startBrowser
call below

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:113: command =
'/usr/bin/google-chrome';
that is not really the default browser, you could try doing xdg-open and then
fall back to using chrome if it does not work

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:117:
Process.run(command, ['$url']);
You may want to get the output and print an error message if we could not launch
it

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/web/display.dart (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:1: import 'dart:html';
missing copyright header

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:56: for (int pos = 0; pos <
source.length; pos++) {
for (var l in source)

[zarah, 2014-05-14 20:51:02]

Thanks!

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/README.TXT (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/README.TXT:1: This program serves a visualization
of a JavaScript source map file generated by dart2js or pub.
On 2014/05/14 12:00:38, Johnni Winther wrote:
> Long line.

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:4: import 'dart:io';
On 2014/05/14 12:00:38, Johnni Winther wrote:
> Add blank line before import. Maybe add library tag.

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:32: return;
On 2014/05/14 12:28:08, ricow1 wrote:
> I would set the exitCode to non zero here - it is good style to give non zero
> exit code in case of failure (same above)

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:51: print("Error: $e");
On 2014/05/14 12:28:08, ricow1 wrote:
> I would indent this 2 

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:71: int port = 9223;
On 2014/05/14 12:28:08, ricow1 wrote:
> I would use an ephemeral port here, you can easily pass that to the
startBrowser
> call below 

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:109: command = 'start';
On 2014/05/14 12:00:38, Johnni Winther wrote:
> Change 'start' to 'cmd.exe /C start'.

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:113: command =
'/usr/bin/google-chrome';
On 2014/05/14 12:28:08, ricow1 wrote:
> that is not really the default browser, you could try doing xdg-open and then
> fall back to using chrome if it does not work

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:117:
Process.run(command, ['$url']);
On 2014/05/14 12:28:08, ricow1 wrote:
> You may want to get the output and print an error message if we could not
launch
> it

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/web/display.dart (right):

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:1: import 'dart:html';
On 2014/05/14 12:28:08, ricow1 wrote:
> missing copyright header

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:56: for (int pos = 0; pos <
source.length; pos++) {
On 2014/05/14 12:28:08, ricow1 wrote:
> for (var l in source)

The variable pos is needed below so this would not simplify the code.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:181:
generatedOutput.children.add(new BRElement());
On 2014/05/14 12:00:38, Johnni Winther wrote:
> Replace `new BRElement()` by `new
> DivElement()..children.add(getLineNumberElement(lineNumber))`.

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:219: div..appendText("
")..children.add(createSpan(token, previous, lineEntry));
On 2014/05/14 12:00:38, Johnni Winther wrote:
> Remove `..appendText(" ")`.

Done.

https://codereview.chromium.org/280513002/diff/130001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/web/display.dart:235: if (entry != null)
addTargetLine(line, target[line], entry); else
On 2014/05/14 12:00:38, Johnni Winther wrote:
> Add {} to the branches.

Done.

[zarah, 2014-05-15 11:48:06]

Committed patchset #2 manually as r36215 (presubmit successful).

[ricow1, 2014-05-15 12:04:55]

https://codereview.chromium.org/280513002/diff/150001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart (right):

https://codereview.chromium.org/280513002/diff/150001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:114: command = 'cmd.exe
/C start';
just out of curiosity, why do we need to do cmd.exe /C

https://codereview.chromium.org/280513002/diff/150001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:118: String xdg =
'/usr/bin/xdg-open';
I don't really mind if you do this, but you could do:
if (Process.runSync('xdg-open', ['--version']).exitCode == 0) {
  command = 'xdg-open';
} else {
  command = '/usr/bin/google-chrome';
}

[Johnni Winther, 2014-05-15 13:07:44]

https://codereview.chromium.org/280513002/diff/150001/tools/dart2js/sourceMap...
File tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart (right):

https://codereview.chromium.org/280513002/diff/150001/tools/dart2js/sourceMap...
tools/dart2js/sourceMapViewer/bin/source_map_viewer.dart:114: command = 'cmd.exe
/C start';
On 2014/05/15 12:04:55, ricow1 wrote:
> just out of curiosity, why do we need to do cmd.exe /C

'start' is not a program but a command in cmd.exe. /C means interpret the rest
as a commandline.