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

Give access to a Flag Span #242

Closed
Ten0 opened this issue Jul 7, 2023 · 6 comments
Closed

Give access to a Flag Span #242

Ten0 opened this issue Jul 7, 2023 · 6 comments

Comments

@Ten0
Copy link
Contributor

Ten0 commented Jul 7, 2023

Sometimes flags are incompatible with others and you'd like to refer to the span of one in the error.
For that purpose it would be useful to get the span at the same time.

impl Flag {
    /// If the flag is present, gives its span. If it's not present, `None`.
    pub fn present_span() -> Option<&Span> {
        self.0.as_ref()
    }
}
@TedDriggs
Copy link
Owner

TedDriggs commented Jul 10, 2023

We could expose an inherent method for this - previously, it was exposed via the Spanned trait but that was broken as part of the syn v2 update (see dtolnay/syn#1441)

If we did this, I wouldn't have it borrow the span - I'd do:

impl Flag {
    pub fn span(&self) -> Span {
        self.0.copied().unwrap_or_else(Span::call_site)
    }
}

@greyblake
Copy link
Contributor

I ran into the similar issue.
I'm trying to get a span out of Flag to generate a custom error message, but there is not way to get it.
The doc for Flag says use

Err(Error::custom("Cannot set flag_a and flag_b").with_span(self.flag_b))

.with_span is based on Spanned trait from syn, but Flag does not implement it in the current version.

@Ten0
Copy link
Contributor Author

Ten0 commented Jul 10, 2023

If we did this, I wouldn't have it borrow the span - I'd do

Oh I wasn't aware Span had Copy. Yeah that seems to be a better interface.

@TedDriggs
Copy link
Owner

@greyblake without the linked syn issue being resolved, using Flag with the errors API is going to be painful, sadly. I suppose it would be possible to incur a breaking change by:

  1. Introduce darling_core::Spanned
  2. Add impl <T: syn::spanned::Spanned> Spanned for T
  3. Update with_span to rely on the darling_core trait instead of the syn version
  4. Add impl darling_core::Spanned for Flag

However, my preference would be that dtolnay/syn#1441 get fixed, so I can simply re-add the Spanned impl to Flag directly.

@greyblake
Copy link
Contributor

@TedDriggs Thank you for the update!

To be honest I am a bit confused what word "sealed" means exactly in this context.
I see that the Spanned trait is available in syn 2: https://docs.rs/syn/latest/syn/spanned/trait.Spanned.html
And many other things based on it are still functioning.

For now I've switched to bool instead of Flag as a workaround to be able to get the proper span (tough I know, it's not exactly the same).

@TedDriggs
Copy link
Owner

Here's an explanation of what "sealed" means.

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

3 participants