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

no member named 'on_error' in 'fmt::basic_format_parse_context<char>' #3529

Closed
mjerabek opened this issue Jul 14, 2023 · 4 comments
Closed

Comments

@mjerabek
Copy link
Contributor

The example for custom formatter specifies that in case of format parse error, one should call ctx.on_error("invalid format"). But this method got removed in d174508 and the impl uses detail::throw_format_error, which should not be used from outsude.

What is the recommended way to report parse errors now?

Works in 9.1.0. Broken in 10.0.0. Still fails on master.

godbolt link: https://godbolt.org/z/dEWEKEsTc

@mjerabek
Copy link
Contributor Author

I worked this around by defining my own throw_format_error function and using that, but I think the lib should expose such a function. One can just throw format_error(...) directly, but then including <fmt/core.h> is not enough (and including <fmt/format.h> in a header is too heavy).

@vitaut
Copy link
Contributor

vitaut commented Jul 14, 2023

Duplicate of #3458.

@mjerabek
Copy link
Contributor Author

Ah, right, didn't find that.

The example is still wrong, though, as now including <fmt/core.h> is not enough. We could change the include in just the one example (as the rest doesn't need <fmt/format.h>).

But IMO having the throwing function available directly from the library (and having the includes consistent) would be less confusing.

I can create a PR with whatever variant you want to use if you want.

@vitaut
Copy link
Contributor

vitaut commented Jul 15, 2023

Good catch, thanks. Fixed the include in 8e87d3a.

A PR to expose throw_format_error via the public API is welcome but note that for ABI compatibility reasons it should be done via an alias rather than moving the function to another namespace.

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

No branches or pull requests

2 participants