Skip to content

Conversation

@bengl
Copy link
Member

@bengl bengl commented Oct 17, 2017

Original PR: #15647

danbev and others added 14 commits October 17, 2017 16:05
Currently the variables set in vcbuild.bat are mostly global and
escape/leak out into the calling process. For example, running
vcbuild.bat test and then echoing the config variable gives:

vcbuild.bat test
...
echo %config%
Release

After this change the same command give:
vcbuild.bat test
...
echo %config%
%config%

PR-URL: nodejs#15754
Reviewed-By: James M Snell <[email protected]>
In code example `vm.createContext` called with new operator by mistake.
It is not a constructor.

PR-URL: nodejs#16059
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
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: nodejs#15771
Refs: jsdom/jsdom#2003
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#16014
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
There are no longer files in the repository that use the `.markdown`
extension so remove mention of them.

PR-URL: nodejs#15786
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Lance Ball <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#15978
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#15972
Reviewed-By: Ruben Bridgewater <[email protected]>
Updated console example to follow style of rest of the examples

PR-URL: nodejs#15962
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: nodejs#15956
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#15954
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#15937
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
No need to require it on each of those function calls.

PR-URL: nodejs#15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@nodejs-github-bot nodejs-github-bot added tty Issues and PRs related to the tty subsystem. v6.x labels Oct 17, 2017
@bengl
Copy link
Member Author

bengl commented Oct 17, 2017

MylesBorins pushed a commit that referenced this pull request Oct 18, 2017
No need to require it on each of those function calls.

Backport-PR-URL: #16264
PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins
Copy link
Contributor

landed in 1d94a86

MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
No need to require it on each of those function calls.

Backport-PR-URL: #16264
PR-URL: #15647
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tty Issues and PRs related to the tty subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.