When we hit a breakpoint in the debugger, show the source of current line.

When we hit a breakpoint in the debugger, show the source of current line.

 - We keep a cache of source & line info in the TargetScript class.

 - We need to use line source more broadly, but ddbg needs to be "futurized"
   first.

Added an uncaught exception handler to the root zone to try to
gracefully close the debugger.

R=hausner@google.com

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

[turnidge, 2013-12-05 23:00:01]

Sample output:

-------------------------

random:dart turnidge$ xcodebuild/DebugIA32/dart tools/ddbg.dart --checked
--package-root=xcodebuild/DebugIA32/packages/ ~/dart2/dummy/bin/dummy.dart
> run
Started process 16256 'xcodebuild/DebugIA32/dart --debug:5858 --checked
--package-root=xcodebuild/DebugIA32/packages/
/Users/turnidge/dart2/dummy/bin/dummy.dart '
Isolate 7112 has been created.
Current isolate set to 7112.
Breakpoint 
    at dummy.dart:5
5         print("Hello");
> s
Hello
6         print("this");
> r
this
is
a
great
example
of
why
this
program
Main isolate 7112 has terminated.
All isolates have terminated.
Process exited normally.
> quit

[hausner, 2013-12-05 23:57:53]

LGTM with musings.

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart
File tools/ddbg.dart (right):

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode986
tools/ddbg.dart:986: return Future.wait([
Your comments helped a lot to understand the control flow and nesting here, so
this is fine. 

OTOH, I'm still trying to experiment with making Futures programming readable.
Do you think this would be more readable:

Future sourceResponse = sendCmd(sourceCmd).then((response) ....);
Future lineResponse = sendCmd(lineNumberCmd).then((response) ...);

return Future.wait([sourceResponse, lineResponse]).then((_) {....};

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode1037
tools/ddbg.dart:1037: return printLocation((stepMode ? null : "Breakpoint"),
location);
A cleaner way would be to add to the wire protocol whether the pause is due to
stepping or due to a breakpoint. I'll file a bug against myself to add that to
the protocol.

[Bob Nystrom, 2013-12-06 00:08:09]

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart
File tools/ddbg.dart (right):

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode986
tools/ddbg.dart:986: return Future.wait([
Drive-by!

> OTOH, I'm still trying to experiment with making Futures programming readable.

It never gets completely readable, it just gets less unreadable. :)

The indentation you're using here doesn't follow the style guide and is probably
not that helpful. Taking Mattias' suggestion and going with how I usually see
this code styled would give you:

var sourceResponse = sendCmd(sourceCmd).then((response) {
  // Send the source command.
  var result = response["result"];
  script.source = result['text'];
  // Line numbers are 1-based so add a dummy for line 0.
  script.lineToSource = [''];
  script.lineToSource.addAll(script.source.split('\n'));
});

var lineResponse = sendCmd(lineNumberCmd).then((response) {
  // Send the line numbers command.
  var result = response["result"];
  script.tokenToLine = parseLineNumberTable(result['lines']);
});

return Future.wait([sourceResponse, lineResponse]).then((_) {
  // When both commands complete, cache the result.
  isolate.scripts[url] = script;
  return script;
});

[turnidge, 2013-12-06 17:19:43]

Thanks for the review, guys.  Bob, thanks for the style tips -- picking it up as
I go.  Through in some bonus indentation changes as well.

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart
File tools/ddbg.dart (right):

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode986
tools/ddbg.dart:986: return Future.wait([
On 2013/12/06 00:08:09, Bob Nystrom wrote:
> Drive-by!
> 
> > OTOH, I'm still trying to experiment with making Futures programming
readable.
> 
> It never gets completely readable, it just gets less unreadable. :)
> 
> The indentation you're using here doesn't follow the style guide and is
probably
> not that helpful. Taking Mattias' suggestion and going with how I usually see
> this code styled would give you:
> 
> var sourceResponse = sendCmd(sourceCmd).then((response) {
>   // Send the source command.
>   var result = response["result"];
>   script.source = result['text'];
>   // Line numbers are 1-based so add a dummy for line 0.
>   script.lineToSource = [''];
>   script.lineToSource.addAll(script.source.split('\n'));
> });
> 
> var lineResponse = sendCmd(lineNumberCmd).then((response) {
>   // Send the line numbers command.
>   var result = response["result"];
>   script.tokenToLine = parseLineNumberTable(result['lines']);
> });
> 
> return Future.wait([sourceResponse, lineResponse]).then((_) {
>   // When both commands complete, cache the result.
>   isolate.scripts[url] = script;
>   return script;
> });

Done.

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode986
tools/ddbg.dart:986: return Future.wait([
On 2013/12/05 23:57:53, hausner wrote:
> Your comments helped a lot to understand the control flow and nesting here, so
> this is fine. 
> 
> OTOH, I'm still trying to experiment with making Futures programming readable.
> Do you think this would be more readable:
> 
> Future sourceResponse = sendCmd(sourceCmd).then((response) ....);
> Future lineResponse = sendCmd(lineNumberCmd).then((response) ...);
> 
> return Future.wait([sourceResponse, lineResponse]).then((_) {....};

Done.

https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode1037
tools/ddbg.dart:1037: return printLocation((stepMode ? null : "Breakpoint"),
location);
On 2013/12/05 23:57:53, hausner wrote:
> A cleaner way would be to add to the wire protocol whether the pause is due to
> stepping or due to a breakpoint. I'll file a bug against myself to add that to
> the protocol.

That would be nice.  Also, it might be helpful to include the breakpoint id when
we hit a breakpoint.  That way it will be easier to say "Breakpoint #2" or
whatever when we stop.

[turnidge, 2013-12-06 17:20:47]

*through* -> *threw*.  There is some deep part of my brain that types sonically.

On 2013/12/06 17:19:43, turnidge wrote:
> Thanks for the review, guys.  Bob, thanks for the style tips -- picking it up
as
> I go.  Through in some bonus indentation changes as well.
> 
> https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart
> File tools/ddbg.dart (right):
> 
>
https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode986
> tools/ddbg.dart:986: return Future.wait([
> On 2013/12/06 00:08:09, Bob Nystrom wrote:
> > Drive-by!
> > 
> > > OTOH, I'm still trying to experiment with making Futures programming
> readable.
> > 
> > It never gets completely readable, it just gets less unreadable. :)
> > 
> > The indentation you're using here doesn't follow the style guide and is
> probably
> > not that helpful. Taking Mattias' suggestion and going with how I usually
see
> > this code styled would give you:
> > 
> > var sourceResponse = sendCmd(sourceCmd).then((response) {
> >   // Send the source command.
> >   var result = response["result"];
> >   script.source = result['text'];
> >   // Line numbers are 1-based so add a dummy for line 0.
> >   script.lineToSource = [''];
> >   script.lineToSource.addAll(script.source.split('\n'));
> > });
> > 
> > var lineResponse = sendCmd(lineNumberCmd).then((response) {
> >   // Send the line numbers command.
> >   var result = response["result"];
> >   script.tokenToLine = parseLineNumberTable(result['lines']);
> > });
> > 
> > return Future.wait([sourceResponse, lineResponse]).then((_) {
> >   // When both commands complete, cache the result.
> >   isolate.scripts[url] = script;
> >   return script;
> > });
> 
> Done.
> 
>
https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode986
> tools/ddbg.dart:986: return Future.wait([
> On 2013/12/05 23:57:53, hausner wrote:
> > Your comments helped a lot to understand the control flow and nesting here,
so
> > this is fine. 
> > 
> > OTOH, I'm still trying to experiment with making Futures programming
readable.
> > Do you think this would be more readable:
> > 
> > Future sourceResponse = sendCmd(sourceCmd).then((response) ....);
> > Future lineResponse = sendCmd(lineNumberCmd).then((response) ...);
> > 
> > return Future.wait([sourceResponse, lineResponse]).then((_) {....};
> 
> Done.
> 
>
https://codereview.chromium.org/106743008/diff/20001/tools/ddbg.dart#newcode1037
> tools/ddbg.dart:1037: return printLocation((stepMode ? null : "Breakpoint"),
> location);
> On 2013/12/05 23:57:53, hausner wrote:
> > A cleaner way would be to add to the wire protocol whether the pause is due
to
> > stepping or due to a breakpoint. I'll file a bug against myself to add that
to
> > the protocol.
> 
> That would be nice.  Also, it might be helpful to include the breakpoint id
when
> we hit a breakpoint.  That way it will be easier to say "Breakpoint #2" or
> whatever when we stop.

[siva, 2013-12-06 17:23:46]

> The indentation you're using here doesn't follow the style guide and is
probably
> not that helpful. Taking Mattias' suggestion and going with how I usually see
> this code styled would give you:
> 
> var sourceResponse = sendCmd(sourceCmd).then((response) {
>   // Send the source command.
>   var result = response["result"];
>   script.source = result['text'];
>   // Line numbers are 1-based so add a dummy for line 0.
>   script.lineToSource = [''];
>   script.lineToSource.addAll(script.source.split('\n'));
> });
> 

I find this indentation also not being very readable, it is hard to figure out
that the code actually belongs to the response handler. The comment 'Send the
source command.' seems out of
place.

I would have preferred:

var sourceResponseHandler = (response) {
  var result = response["result"];
  script.source = result['text'];
  // Line numbers are 1-based so add a dummy for line 0.
  script.lineToSource = [''];
  script.lineToSource.addAll(script.source.split('\n'));
};

// Send the source command and set up a handler for the asynchronous response.
var sourceResponse = sendCmd(sourceCmd).then(sourceResponseHandler);

[turnidge, 2013-12-06 17:28:21]

Committed patchset #6 manually as r30953 (presubmit successful).