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

Improve warn behavior to return Error instead of printing to stderr #93

Open
ankitpatel96 opened this issue Mar 11, 2021 · 8 comments
Open

Comments

@ankitpatel96
Copy link

Hi,
I was wondering if we could improve the behavior of chevron.render (with warn=True) so that it hands back an error type containing the issues along with the rendered string, rather than just printing to stderr? This would make it easier to use the library programmatically. This seems like a breaking api change so switching to a different method or cutting a new major release would probably be necessary. I'm open to submitting a PR for this if you're open to this change.

@noahmorrison
Copy link
Owner

Yeah this sounds reasonable. I guess the way to do this would be to rename and change the current function to return an Error on failures, and replace the function with one that calls it, and on error prints it to stderr?

I might try to write this up this weekend, but if I don't feel free to beat me to it with your own PR!

@dmorrison42
Copy link
Contributor

I'm pretty sure we made this backward compatible!

If this looks good we can merge it in.

@noahmorrison
Copy link
Owner

noahmorrison commented Mar 20, 2021

Here's the relevant commit

With a test case here

If this satisfies what you wanted I can merge it in.

@jonhartnett
Copy link

Can't speak for the OP, but this commit solves my issue. Currently I'm intercepting stderr to detect warnings, but I'd love to switch to something more robust.

@ex3cv
Copy link

ex3cv commented May 10, 2021

Hi @noahmorrison. Do you see a chance to merge this any time soon and make a release?

@stefanpl
Copy link

Hey guys 👋

May I kindly ask what's the status here? To me it looks like there's a mergeable solution ready, so why not push it across the finish line?

@noahmorrison would you be so kind? 😊

@ankitpatel96
Copy link
Author

ankitpatel96 commented Mar 25, 2022

Sorry I switched to another project at work but I 100% support just putting up a PR with this branch! The change is pretty clever and would totally work for my use case. I'd gladly review and approve @noahmorrison's change.

@gl-yziquel
Copy link

On this cloned repo https://github.com/gl-yziquel/chevron you may pass a function to the warn argument, and, in the case of a missing key, the missing key will be passed to the callable warn. And what the function returns will be what will be interpolated.

Fail instead of returning something, if you want...

Just stating this for anyone who wishes an off the shelf solution.

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

No branches or pull requests

7 participants