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.

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

util: hide toStringTags if already included in the constructor #28207

Closed

Conversation

BridgeAR
Copy link
Member

This makes sure that Symbol.toStringTag entries won't be taken
into account when inspecting objects with util.inspect() in case
the tag is already included in the found constructor name.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

This makes sure that `Symbol.toStringTag` entries won't be taken
into account when inspecting objects with `util.inspect()` in case
the tag is already included in the found constructor name.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 13, 2019

Sadly, an error occurred when I tried to trigger a build. :(
CI: https://ci.nodejs.org/job/node-test-pull-request/23866/

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Jun 13, 2019
@Trott
Copy link
Member

Trott commented Jun 13, 2019

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 13, 2019
@TimothyGu
Copy link
Member

I’m not convinced this is a good idea. This would elide the toStringTag even for completely irrelevant class names where toStringTag would be helpful, like Maple [Map].

@BridgeAR
Copy link
Member Author

@TimothyGu that is a very valid point. I could verify that a following letter is a capital letter.

@TimothyGu
Copy link
Member

@BridgeAR

that is a very valid point. I could verify that a following letter is a capital letter.

I don't think that's good either. What if the following code point is a number? an underscore? Chinese character? Coptic numeral?

In my view, doing anything other than the current one makes the function even more opinionated than it is, and also brings tremendous complexity to a function already touching on the boundaries of what we stand for as a "small core."

@BridgeAR
Copy link
Member Author

I don't have a strong opinion on this, it was just a thought to reduce duplication.
Thanks for the feedback.

@BridgeAR BridgeAR closed this Jun 14, 2019
@BridgeAR BridgeAR deleted the hide-tags-similar-to-constructor branch January 20, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants