Skip to content
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

Jump to bottom

util,src: allow lookup of hidden values #3988

Merged
merged 0 commits into from
Nov 24, 2015
Merged

util,src: allow lookup of hidden values #3988

merged 0 commits into from
Nov 24, 2015

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Nov 23, 2015

This commit adds an internal util method that makes hidden values in the C++ layer visible in JS.

I'd like to use this to more easily access the hidden arrowMessage associated with syntax errors. Refs #3784, #3411, #2762

@mscdex mscdex added util Issues and PRs related to the built-in util module. c++ Issues and PRs that require attention from people who are familiar with C++. labels Nov 23, 2015
trevnorris
trevnorris reviewed Nov 23, 2015
View reviewed changes
src/node_util.cc
return env->ThrowTypeError("name must be a string");

Local<Object> obj = args[0]->ToObject(env->isolate());
Local<String> name = args[1]->ToString(env->isolate());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you've already verified the type then simply use .As<TYPE>()

@trevnorris
Copy link
Contributor

one comment, and if CI is happy then LGTM.

@bnoordhuis
Copy link
Member

LGTM with the same comment as Trevor.

@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 24, 2015

CI: https://ci.nodejs.org/job/node-test-pull-request/833/

UPDATE: CI looked like it had some unrelated issues. One more run, just to be sure: https://ci.nodejs.org/job/node-test-pull-request/836/

@cjihrig cjihrig closed this Nov 24, 2015
cjihrig added a commit to cjihrig/node that referenced this pull request Nov 24, 2015
@cjihrig
util,src: allow lookup of hidden values
5169311 
This commit adds an internal util method that makes hidden
values in the C++ layer visible in JS.

PR-URL: nodejs#3988
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@cjihrig cjihrig merged commit 5169311 into nodejs:master Nov 24, 2015
@cjihrig cjihrig deleted the hidden branch November 24, 2015 20:50
@Fishrock123 Fishrock123 mentioned this pull request Nov 26, 2015
Closed
@jasnell
Copy link
Member

jasnell commented Nov 30, 2015

@cjihrig ... is this a candidate for LTS?

@cjihrig cjihrig mentioned this pull request Nov 30, 2015
Merged
@cjihrig
Copy link
Contributor Author

cjihrig commented Nov 30, 2015

This PR alone is of little use in LTS. It only introduces a new internal method. It should be grouped in with #4013.

@rvagg
Copy link
Member

rvagg commented Dec 5, 2015

why is github showing zero commits and zero changes here? I believe that 5169311 on master comes from here but it's hard to be sure.

@jasnell
Copy link
Member

jasnell commented Dec 5, 2015

Oh interesting. That is very strange.

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 5, 2015

Seems to be a result of landing a commit and then force pushing this branch to close the PR. Probably shouldn't do that anymore.

cjihrig added a commit that referenced this pull request Dec 5, 2015
@cjihrig @rvagg
util,src: allow lookup of hidden values
c584c3e 
This commit adds an internal util method that makes hidden
values in the C++ layer visible in JS.

PR-URL: #3988
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This was referenced Dec 8, 2015
Closed
Merged
@rvagg rvagg mentioned this pull request Dec 17, 2015
Closed
@MylesBorins MylesBorins mentioned this pull request Dec 30, 2015
Closed
cjihrig added a commit to cjihrig/node that referenced this pull request Jan 7, 2016
@cjihrig
util,src: allow lookup of hidden values
a1f4285 
This commit adds an internal util method that makes hidden
values in the C++ layer visible in JS.

PR-URL: nodejs#3988
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	lib/internal/util.js
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
@cjihrig
util,src: allow lookup of hidden values
1bf84b9 
This commit adds an internal util method that makes hidden
values in the C++ layer visible in JS.

PR-URL: #3988
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Conflicts:
	lib/internal/util.js
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
2016-01-19, Version 4.2.5 "Argon" (LTS) Release
2b6386a 
Notable changes:

* assert
  -  accommodate ES6 classes that extend Error (Rich Trott) #4166
* build
  - add "--partly-static" build options (Super Zheng) #4152
* deps
  - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655
  - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128
  - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé)
* docs
  - various updates landed in 70 different commits!
* repl
  - attach location info to syntax errors (cjihrig) #4013
  - display error message when loading directory (Prince J Wesley) #4170
* tests
  - various updates landed in over 50 commits
* util
  - allow lookup of hidden values (cjihrig) #3988
MylesBorins pushed a commit that referenced this pull request Jan 20, 2016
2016-01-20, Version 4.2.5 "Argon" (LTS) Release
e6ca04c 
Notable changes:

* assert
  -  accommodate ES6 classes that extend Error (Rich Trott) #4166
* build
  - add "--partly-static" build options (Super Zheng) #4152
* deps
  - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655
  - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128
  - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé)
* docs
  - various updates landed in 70 different commits!
* repl
  - attach location info to syntax errors (cjihrig) #4013
  - display error message when loading directory (Prince J Wesley) #4170
* tests
  - various updates landed in over 50 commits
* util
  - allow lookup of hidden values (cjihrig) #3988

PR-URL: #4768
MylesBorins pushed a commit that referenced this pull request Jan 20, 2016
2016-01-20, Version 4.2.5 "Argon" (LTS) Release
d50a8a9 
Notable changes:

* assert
  -  accommodate ES6 classes that extend Error (Rich Trott) #4166
* build
  - add "--partly-static" build options (Super Zheng) #4152
* deps
  - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655
  - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128
  - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé)
* docs
  - various updates landed in 70 different commits!
* repl
  - attach location info to syntax errors (cjihrig) #4013
  - display error message when loading directory (Prince J Wesley) #4170
* tests
  - various updates landed in over 50 commits
* util
  - allow lookup of hidden values (cjihrig) #3988

PR-URL: #4768
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
2016-01-20, Version 4.2.5 "Argon" (LTS) Release
028e811 
Notable changes:

* assert
  -  accommodate ES6 classes that extend Error (Rich Trott) nodejs#4166
* build
  - add "--partly-static" build options (Super Zheng) nodejs#4152
* deps
  - backport 066747e from upstream V8 (Ali Ijaz Sheikh) nodejs#4655
  - backport 200315c from V8 upstream (Vladimir Kurchatkin) nodejs#4128
  - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé)
* docs
  - various updates landed in 70 different commits!
* repl
  - attach location info to syntax errors (cjihrig) nodejs#4013
  - display error message when loading directory (Prince J Wesley) nodejs#4170
* tests
  - various updates landed in over 50 commits
* util
  - allow lookup of hidden values (cjihrig) nodejs#3988

PR-URL: nodejs#4768
This was referenced Nov 29, 2023
Open
Open
This was referenced May 14, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cjihrig @trevnorris @bnoordhuis @jasnell @rvagg @mscdex @MylesBorins