src: make source list -l flag optional

`v8 source list` was expecting an undocumented flag -l, which is the
first line the command will output from source code. This commit makes
that flag optional, documents the flag, improves the help message for
this command and adds tests for it.

Fixes: #138

PR-URL: #259
Reviewed-By: Joyee Cheung <[email protected]>

Author: mmarchini

README.md

@@ -262,7 +262,11 @@ The following subcommands are supported:
       print           -- Print short description of the JavaScript value.
 
                          Syntax: v8 print expr
-      source          -- Source code information
+      source list     -- Print source lines around the currently selected
+                         JavaScript frame.
+                         Syntax: v8 source list [flags]
+                         Flags:
+                         * -l <line> - Print source code below line <line>.
 
 For more help on any particular subcommand, type 'help <command> <subcommand>'.
 ```

src/llnode.cc

@@ -194,11 +194,6 @@ bool PrintCmd::DoExecute(SBDebugger d, char** cmd,
 
 bool ListCmd::DoExecute(SBDebugger d, char** cmd,
                         SBCommandReturnObject& result) {
-  if (cmd == nullptr || *cmd == nullptr) {
-    result.SetError("USAGE: v8 source list\n");
-    return false;
-  }
-
   static SBFrame last_frame;
   static uint64_t last_line = 0;
   SBTarget target = d.GetSelectedTarget();
@@ -212,22 +207,24 @@ bool ListCmd::DoExecute(SBDebugger d, char** cmd,
   bool grab_line = false;
   bool line_switch = false;
   int line_from_switch = 0;
-  for (char** start = cmd; *start != nullptr; start++) {
-    if (grab_line) {
-      grab_line = false;
-      line_switch = true;
-      errno = 0;
-      line_from_switch = strtol(*start, nullptr, 10);
-      if (errno) {
-        result.SetError("Invalid line number");
-        return false;
+  if (cmd != nullptr) {
+    for (char** start = cmd; *start != nullptr; start++) {
+      if (grab_line) {
+        grab_line = false;
+        line_switch = true;
+        errno = 0;
+        line_from_switch = strtol(*start, nullptr, 10);
+        if (errno) {
+          result.SetError("Invalid line number");
+          return false;
+        }
+        line_from_switch--;
       }
-      line_from_switch--;
-    }
-    if (strcmp(*start, "-l") == 0) {
-      grab_line = true;
+      if (strcmp(*start, "-l") == 0) {
+        grab_line = true;
+      }
+      full_cmd += *start;
     }
-    full_cmd += *start;
   }
   if (grab_line || (line_switch && line_from_switch < 0)) {
     result.SetError("Expected line number after -l");
@@ -434,8 +431,12 @@ bool PluginInitialize(SBDebugger d) {
   SBCommand source =
       v8.AddMultiwordCommand("source", "Source code information");
   source.AddCommand("list", new llnode::ListCmd(&llv8),
-                    "Print source lines around a selected JavaScript frame.\n\n"
-                    "Syntax: v8 source list\n");
+                    "Print source lines around the currently selected "
+                    "JavaScript frame.\n\n"
+                    "Syntax: v8 source list [flags]\n\n"
+                    "Flags:\n"
+                    " * -l <line> - Print source code below line <line>.\n"
+                    );
   interpreter.AddCommand("jssource", new llnode::ListCmd(&llv8),
                          "Alias for `v8 source list`");
 

test/plugin/frame-test.js

@@ -4,6 +4,62 @@ const tape = require('tape');
 
 const common = require('../common');
 
+const sourceCode = [
+"10 function eyecatcher() {",
+"11   crasher();    // # args < # formal parameters inserts an adaptor frame.",
+"12   return this;  // Force definition of |this|.",
+"13 }",
+];
+const lastLine = new RegExp(sourceCode[sourceCode.length - 1]);
+
+function fatalError(t, sess, err) {
+  t.error(err);
+  sess.quit();
+  return t.end();
+}
+
+function testFrameList(t, sess, frameNumber) {
+  sess.send(`frame select ${frameNumber}`);
+  sess.linesUntil(/frame/, (err, lines) => {
+    if (err) {
+      return fatalError(t, sess, err);
+    }
+    sess.send('v8 source list');
+
+    sess.linesUntil(/v8 source list/, (err, lines) => {
+      sess.linesUntil(lastLine, (err, lines) => {
+        if (err) {
+          return fatalError(t, sess, err);
+        }
+        t.equal(lines.length, sourceCode.length,
+                `v8 source list correct size`);
+        for (let i = 0; i < lines.length; i++) {
+          t.equal(lines[i].trim(), sourceCode[i], `v8 source list #${i}`);
+        }
+
+        sess.send('v8 source list -l 2');
+
+        sess.linesUntil(/v8 source list/, (err, lines) => {
+          sess.linesUntil(lastLine, (err, lines) => {
+            if (err) {
+              return fatalError(t, sess, err);
+            }
+            t.equal(lines.length, sourceCode.length - 1,
+                    `v8 source list -l 2 correct size`);
+            for (let i = 0; i < lines.length; i++) {
+              t.equal(lines[i].trim(), sourceCode[i + 1],
+                      `v8 source list -l 2 #${i}`);
+            }
+
+            sess.quit();
+            t.end();
+          });
+        });
+      });
+    });
+  });
+}
+
 tape('v8 stack', (t) => {
   t.timeoutAfter(15000);
 
@@ -43,7 +99,13 @@ tape('v8 stack', (t) => {
     // The test adds unreachable `return this` statements as a workaround.
     t.ok(/this=(0x[0-9a-f]+):<Global proxy>/.test(eyecatcher), 'global this');
     t.ok(/this=(0x[0-9a-f]+):<undefined>/.test(crasher), 'undefined this');
-    sess.quit();
-    t.end();
+
+    const eyecatcherFrame = eyecatcher.match(/frame #([0-9]+)/)[1];
+    if (!eyecatcherFrame) {
+      fatalError(t, sess, "Couldn't determine eyecather's frame number");
+    }
+
+    testFrameList(t, sess, eyecatcherFrame);
+
   });
 });

test/plugin/usage-test.js

@@ -20,13 +20,6 @@ tape('usage messages', (t) => {
     t.error(err);
     const re = /^error: USAGE: v8 print expr$/;
     t.ok(containsLine(lines, re), 'print usage message');
-    sess.send('v8 source list');
-  });
-
-  sess.stderr.linesUntil(/USAGE/, (err, lines) => {
-    t.error(err);
-    const re = /^error: USAGE: v8 source list$/;
-    t.ok(containsLine(lines, re), 'list usage message');
     sess.send('v8 findjsinstances');
   });