Skip to content

[v6.x backport] tty: require readline at top of file #16264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bengl wants to merge 14 commits into from
Closed

[v6.x backport] tty: require readline at top of file #16264

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b8ab430
build: call setlocal in vcbuild.bat
danbev Oct 2, 2017
3991953
doc: fix incorrect vm.createContext usage
tshemsedinov Oct 7, 2017
c405f95
src: fix ^ in stack trace with vm's columnOffset
TimothyGu Oct 4, 2017
c81b425
test: remove messages in assert.strictEqual
Saeed-Navarik Oct 6, 2017
6d49154
doc: update style guide for markdown extension
Trott Oct 5, 2017
d0157b7
test: alter assert.strictEqual to default message
gishmel Oct 6, 2017
0c0717c
test: replacing assert message with template
Oct 6, 2017
bf1bf28
test: improve assert messages
ericljpemberton Oct 6, 2017
d3544c1
doc: add clearer setup description
craftninja Oct 6, 2017
b43fed4
test: replace literal with template string
brantphoto Oct 6, 2017
97cf4f1
tools: use template literals
sarahmeyer Oct 6, 2017
a686f9c
test: removed string from assert message arg
dpaulino Oct 6, 2017
b93842a
test: changes to use template literal
joanne-jjb Oct 6, 2017
5dc4b25
tty: require readline at top of file
bengl Sep 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
src: fix ^ in stack trace with vm's columnOffset 
While VM module's columnOffset option does succeed in applying an offset
to the column number in the stack trace, the wavy diagram printed does
not account for potential offsets, resulting in erroneous location of
`^` in the first line of the script.

Before:

```
> vm.runInThisContext('throw new Error()', { columnOffset: 5 })
evalmachine.<anonymous>:1
throw new Error()
     ^

Error
    at evalmachine.<anonymous>:1:12
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
```

After:

```
> vm.runInThisContext('throw new Error()', { columnOffset: 5 })
evalmachine.<anonymous>:1
throw new Error()
^

Error
    at evalmachine.<anonymous>:1:12
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at repl:1:4
```

PR-URL: #15771
Refs: jsdom/jsdom#2003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
@TimothyGu @MylesBorins
TimothyGu authored and MylesBorins committed Oct 17, 2017
commit c405f95862bb024313251c1f67350b158eb9fc8c
9 changes: 9 additions & 0 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,7 @@ void AppendExceptionLine(Environment* env,
}

// Print (filename):(line number): (message).
ScriptOrigin origin = message->GetScriptOrigin();
node::Utf8Value filename(env->isolate(), message->GetScriptResourceName());
const char* filename_string = *filename;
int linenum = message->GetLineNumber();
Expand Down Expand Up @@ -1613,8 +1614,16 @@ void AppendExceptionLine(Environment* env,
// sourceline to 78 characters, and we end up not providing very much
// useful debugging info to the user if we remove 62 characters.

int script_start =
(linenum - origin.ResourceLineOffset()->Value()) == 1 ?
origin.ResourceColumnOffset()->Value() : 0;
int start = message->GetStartColumn(env->context()).FromMaybe(0);
int end = message->GetEndColumn(env->context()).FromMaybe(0);
if (start >= script_start) {
CHECK_GE(end, start);
start -= script_start;
end -= script_start;
}

char arrow[1024];
int max_off = sizeof(arrow) - 2;
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-vm-context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,14 @@ assert.strictEqual(script.runInContext(ctx), false);
// Error on the first line of a module should
// have the correct line and column number
assert.throws(() => {
vm.runInContext('throw new Error()', context, {
vm.runInContext(' throw new Error()', context, {
filename: 'expected-filename.js',
lineOffset: 32,
columnOffset: 123
});
}, (err) => {
return /expected-filename\.js:33:130/.test(err.stack);
return /^ \^/m.test(err.stack) &&
/expected-filename\.js:33:131/.test(err.stack);
}, 'Expected appearance of proper offset in Error stack');

// https://github.com/nodejs/node/issues/6158
Expand Down