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

feat: expose parser #207

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bertho-zero
Copy link

This PR exposes the parser as a non-enumerable property.

@alexei
Copy link
Owner

alexei commented Apr 20, 2021

Hey Kévin, thanks for your input. I'd appreciate some clarifications about this change.

@bertho-zero
Copy link
Author

I forked VError to be browser compatible, so I replaced joyent/node-extsprintf with sprintf-js but I can't seem to reproduce the SError class without this change.

SError, which is just like VError but interprets printf-style arguments more strictly.

SError throw an error when using a %s token with null or undefined, for example:

sprintf('%s', null) // throw an error

For that it is necessary to parse the tokens before calling sprintf-js.

@alexei
Copy link
Owner

alexei commented Apr 23, 2021

Oh, I see. This has been asked before (i.e. supporting relaxed, strict modes) and while I agree to the idea, I'm against such approaches where the (undocumented) internals get exposed. At one point I suggested proxying arguments, but I guess that would only work with named placeholders where you pass an object.

@alexei
Copy link
Owner

alexei commented Apr 23, 2021

See also #165

@bertho-zero
Copy link
Author

I don't see how to check the arguments without using the ast that comes out of the parser, it seems normal to me to export it and document it for more advanced use cases like this.

@alexei
Copy link
Owner

alexei commented Apr 23, 2021

It's possible I didn't understand your use case. TBH I don't even know what SError and VError are. But if you want it to not raise on missing argument, you can use a proxy with named placeholders and have the proxy return something on missing key.

@bertho-zero
Copy link
Author

bertho-zero commented Apr 23, 2021

VError is a class that extends native Error and uses sprintf to construct its message. SError should do the same thing but stricter with strings. The documentation is available here, I invite you to take a tour to better understand.

To know if a token is %s I need to parse the string, I don't see how the placeholders could help me, the string comes from the developer who uses VError, not from me, I have no control over the placeholders.

@bertho-zero
Copy link
Author

bertho-zero commented Jul 16, 2021

What is the inconvenience of exposing the parser?

How can I parse the arguments to do some preprocessing before calling sprintf, without duplicating the code of the parser part?

@alexei
Copy link
Owner

alexei commented Jul 17, 2021

What is the inconvenience of exposing the parser?

Probably none, if you do it intentionally. That is not the case with this library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants