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

Add support for paths with angle brackets #122

Merged
merged 4 commits into from
Oct 15, 2023

Conversation

aumetra
Copy link
Contributor

@aumetra aumetra commented Oct 14, 2023

This PR adds support for paths of the build_method(into) field that contain angle brackets (such as Arc<Foo>)

@idanarye
Copy link
Owner

I'm not a fan of Either (and certainly not of unwrap_left - panics make messy compilation errors). I think a better solution would be to make KeyValue::value a verbatim - a TokenStream with no structure behind tokenization and grouping. Settings that don't need to inspect the internal structure of their value can pass it as is, and the ones that do can just parse it on the spot. KeyValue can even have a convenience parse_value function to streamline the syntax.

This would even make existing implementations simpler. For example, instead of this:

"vis" => {
    let value = expr.key_value()?.value;
    let syn::Expr::Lit(expr_lit) = value else {
        return Err(Error::new_spanned(value, "invalid visibility found"));
    };
    let syn::Lit::Str(expr_str) = expr_lit.lit else {
        return Err(Error::new_spanned(expr_lit, "invalid visibility found"));
    };
    self.vis = Some(syn::parse_str(&expr_str.value())?);
    Ok(())
}

We would have this:

"vis" => {
    let value = expr.key_value()?.parse_value::<syn::LitStr>()?.value();
    self.vis = Some(syn::parse_str(value)?);
    Ok(())
}

If this seems too much, I'm willing to just merge this and then do these changes myself.

@aumetra
Copy link
Contributor Author

aumetra commented Oct 15, 2023

The issue here is that a .parse() call on a TokenStream apparently consumes the source ParseStream beyond its punctuated boundaries (for some reason)

Not quite sure why though, as I'm not really an expert when it comes to proc macros

@aumetra
Copy link
Contributor Author

aumetra commented Oct 15, 2023

Just the transform element is being problematic at the moment, the rest is working just fine

@idanarye
Copy link
Owner

Yea. I guess my assumption about all commas hidden inside token trees was incorrect. And now that I think about it - it would also be a problem for generic (< and > are also not creating token groups)

I'll fix it myself after merging.

@idanarye idanarye merged commit 370fdd7 into idanarye:master Oct 15, 2023
5 of 9 checks passed
@aumetra aumetra deleted the angle-bracket-path branch October 15, 2023 15:20
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