Skip to content

Conversation

targos
Copy link
Member

@targos targos commented Dec 11, 2017

Fixes: #17605

Still needs documentation change. I'm opening this for discussion.

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
Affected core subsystem(s)

util

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Dec 11, 2017
@bnoordhuis
Copy link
Member

Why not simply allow a regular expression? More flexible and you don't have to write an ad hoc parser for it.

@targos
Copy link
Member Author

targos commented Dec 11, 2017

Why not simply allow a regular expression?

That would be fine for me too.

@starkwang
Copy link
Contributor

starkwang commented Dec 11, 2017

Why not simply allow a regular expression?

Importing the usage of regular expression may add some complexity and logic confilcts. For example, what will happen if some of section names are contradictory?

--NODE_DEBUG=fs,/^((?!fs).)*$/

The second regular expression matches the string not containing "fs", which is contradictory to the first argument.

@bnoordhuis
Copy link
Member

Simple solution: don't write a broken regex. Garbage in, garbage out.

@targos
Copy link
Member Author

targos commented Dec 12, 2017

Closing in favor of #17609

@targos targos closed this Dec 12, 2017
@targos targos deleted the fix-17605 branch January 2, 2018 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants