frog: life is better with colors :)

frog: life is better with colors :)

Also adds 'Compilation succeded' message when running the code right after it is
compiled.

BUG = http://code.google.com/p/dart/issues/detail?id=708

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

[Bob Nystrom, 2011-12-06 22:47:43]

Minor nit. I'll defer to John for the actual review.

http://codereview.chromium.org/8823010/diff/2001/frog/frog_options.dart
File frog/frog_options.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/frog_options.dart#newco...
frog/frog_options.dart:138: continue loop;
Is this still needed? I believe break is working now.

http://codereview.chromium.org/8823010/diff/2001/frog/world.dart
File frog/world.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/world.dart#newcode413
frog/world.dart:413: ? '${_RED_COLOR}error$_NO_COLOR: $message' : 'error:
$message',
This seems a bit redundant. How about having _message look at the options itself
so you can just do:

_message(_RED_COLOR, 'error', message, span, span1, span2,
options.throwOnErrors);

[Jennifer Messerly, 2011-12-06 22:50:03]

http://codereview.chromium.org/8823010/diff/2001/frog/minfrog.dart
File frog/minfrog.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/minfrog.dart#newcode21
frog/minfrog.dart:21: if (options.outfile !== null) {
I won't add this message unless doing "compileOnly"

if you're just running a Dart script, it's important not to output extra stuff.
Imagine if every Python script printed "compilation succeeded" before running :)

so something like:
if (options.compileOnly) {
  // print compilation succeeded message
} else {
  var code = world.getGeneratedCode();
  process.argv = [argv[0], argv[1]];
  // ... vm.runInNewContext ...
}

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart
File frog/source.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart#newcode80
frog/source.dart:80: var buf = new StringBuffer('${filename}:${line +
1}:${column + 1}: $message');
long line

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart#newcode105
frog/source.dart:105: for (; i < toColumn; i++) {
Would it be nice to display the arrows even in color mode? I'm thinking if I get
a log file some day from a dumb terminal or something. But I'm not sure.

[Bob Nystrom, 2011-12-06 23:12:46]

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart
File frog/source.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart#newcode105
frog/source.dart:105: for (; i < toColumn; i++) {
On 2011/12/06 22:50:03, John Messerly wrote:
> Would it be nice to display the arrows even in color mode? I'm thinking if I
get
> a log file some day from a dumb terminal or something. But I'm not sure.

+1 for always showing ^^^^

[Siggi Cherem (dart-lang), 2011-12-06 23:37:38]

http://codereview.chromium.org/8823010/diff/2001/frog/frog_options.dart
File frog/frog_options.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/frog_options.dart#newco...
frog/frog_options.dart:138: continue loop;
On 2011/12/06 22:47:43, Bob Nystrom wrote:
> Is this still needed? I believe break is working now.

Done.

http://codereview.chromium.org/8823010/diff/2001/frog/minfrog.dart
File frog/minfrog.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/minfrog.dart#newcode21
frog/minfrog.dart:21: if (options.outfile !== null) {
On 2011/12/06 22:50:03, John Messerly wrote:
> I won't add this message unless doing "compileOnly"
> 
> if you're just running a Dart script, it's important not to output extra
stuff.
> Imagine if every Python script printed "compilation succeeded" before running
:)
> 
> so something like:
> if (options.compileOnly) {
>   // print compilation succeeded message
> } else {
>   var code = world.getGeneratedCode();
>   process.argv = [argv[0], argv[1]];
>   // ... vm.runInNewContext ...
> }

The other opinion I heard earlier today was that people wanted to see clearly
when did compilation finished and we started running the actual script (so it is
clear why something failed).

I followed your suggestion for now. Another option is to print this message in 2
scenarios:
- compile only
- compile + run, but outfile was specified.

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart
File frog/source.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart#newcode80
frog/source.dart:80: var buf = new StringBuffer('${filename}:${line +
1}:${column + 1}: $message');
On 2011/12/06 22:50:03, John Messerly wrote:
> long line

Done.

http://codereview.chromium.org/8823010/diff/2001/frog/source.dart#newcode105
frog/source.dart:105: for (; i < toColumn; i++) {
On 2011/12/06 23:12:46, Bob Nystrom wrote:
> On 2011/12/06 22:50:03, John Messerly wrote:
> > Would it be nice to display the arrows even in color mode? I'm thinking if I
> get
> > a log file some day from a dumb terminal or something. But I'm not sure.
> 
> +1 for always showing ^^^^

I added them back, but with colors too. 

I was not convinced of this, because in these cases we shouldn't use colors at
all (use the --no_colors option). Otherwise, the color-escape chars are visible
on plain text. For instance,

code code\033[31merror error\033[0m...
         ^^^^^^^

I made the ^^ colored, so that it aligns well with the error:
code code\033[31merror error\033[0m...
         \033[31m^^^^^^^^^^^\033[0m

http://codereview.chromium.org/8823010/diff/2001/frog/world.dart
File frog/world.dart (right):

http://codereview.chromium.org/8823010/diff/2001/frog/world.dart#newcode413
frog/world.dart:413: ? '${_RED_COLOR}error$_NO_COLOR: $message' : 'error:
$message',
On 2011/12/06 22:47:43, Bob Nystrom wrote:
> This seems a bit redundant. How about having _message look at the options
itself
> so you can just do:
> 
> _message(_RED_COLOR, 'error', message, span, span1, span2,
> options.throwOnErrors);

Done.

[Jennifer Messerly, 2011-12-06 23:42:10]

lgtm