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

assert: accept undefined as message input #22696

Closed
wants to merge 2 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Sep 4, 2018

This is a follow up PR for #22695 but this is currently blocked by #22694. I'll remove the first commit as soon as #22695 lands.

This is important to allow any input value to be visible to the users
in case the message is used to highlight the failing passed through
input.

E.g., assert(/abc/.test(input), input) should highlight, that input
is set to undefined.

Refs: #22695, #22694

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

So far it was difficult to tell what the actual error was about if
there was only the user error. To overcome this, the user message
and the auto-generated message will now be visible at the same time.
This is important to allow any input value to be visible to the users
in case the message is used to highlight the failing passed through
input.

E.g., `assert(/abc/.test(input), input)` should highlight, that `input`
is set to `undefined`.
@BridgeAR BridgeAR added semver-major PRs that contain breaking changes and should be released in the next major version. blocked PRs that are blocked by other issues or PRs. labels Sep 4, 2018
@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Sep 4, 2018
-->
* `options` {Object}
* `message` {string} If provided, the error message is going to be set to this
value.
* `message` {any} If provided, it is going to be appended to auto-generated
Copy link
Member

Choose a reason for hiding this comment

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

going to be -> will be

Copy link
Member

Choose a reason for hiding this comment

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

to auto-generated -> to the

* `userMessage` {any} Contains the actual passed through message by the user.
It will not be set in case the user does not provide a error message.

All error messages thrown by the `assert` module are auto-generated. If the user
Copy link
Member

Choose a reason for hiding this comment

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

This sentence is confusing to me. I don't think auto-generated is a clear term and should probably be avoided.

@nodejs/documentation

@BridgeAR
Copy link
Member Author

Closing for now. I'll open a new PR when I get to it.

@BridgeAR BridgeAR closed this Dec 19, 2018
@BridgeAR BridgeAR deleted the add-undefined-assert-message branch January 20, 2020 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. blocked PRs that are blocked by other issues or PRs. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants