Skip to content

util,src: allow lookup of hidden values - #3988

Merged
cjihrig merged 0 commit into
nodejs:masterfrom
cjihrig:hidden
Nov 24, 2015
Merged

cjihrig merged 0 commit into
nodejs:masterfrom
cjihrig:hidden

Conversation

@cjihrig

@cjihrig cjihrig commented Nov 23, 2015

Copy link
Copy Markdown
Contributor

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
Comment thread src/node_util.cc Outdated

ghost Nov 23, 2015

Copy link
Copy Markdown
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

ghost commented Nov 23, 2015

Copy link
Copy Markdown
Contributor

one comment, and if CI is happy then LGTM.

@bnoordhuis

ghost commented Nov 24, 2015

Copy link
Copy Markdown
Member

LGTM with the same comment as Trevor.

@cjihrig

ghost commented Nov 24, 2015

Copy link
Copy Markdown
Contributor Author

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
cjihrig merged commit 5169311 into nodejs:master Nov 24, 2015
@cjihrig
cjihrig deleted the hidden branch November 24, 2015 20:50
@jasnell

ghost commented Nov 30, 2015

Copy link
Copy Markdown
Member

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

@cjihrig

ghost commented Nov 30, 2015

Copy link
Copy Markdown
Contributor Author

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

@rvagg

ghost commented Dec 5, 2015

Copy link
Copy Markdown
Member

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

ghost commented Dec 5, 2015

Copy link
Copy Markdown
Member

Oh interesting. That is very strange.

@cjihrig

ghost commented Dec 5, 2015

Copy link
Copy Markdown
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

7 participants