-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
Change clone/closure macro syntax to play better with rustfmt and other tooling #1394
Comments
I'll probably look into this during the hackfest. In the meantime if anybody has syntax suggestions... |
@felinira and I played around with this ages ago. I think that option c is a bit special but also much faster to type for me. let x = clone!(
// a
#[weak(x)]
#[strong(self, rename_to = "y")]
// b
#[weak]
x,
#[stong]
self as y,
// c
&'weak x,
self as &'strong y,
// return
// a
#[default_return(false)]
// b
#[default_return]
false,
// c
return false,
move |a, b, c| {
print!("{a} {b} {c} {x} {y}");
}
); For completeness: This one works as well. But I think it might be a bit confusing. let x = clone!(
#[return_default]
true,
move |#[weak] x, #[strong] y: self, a, b, c| {
print!("{a} {b} {c} {x} {y}");
}
); |
Interesting ideas, thanks! Just some initial impression / thoughts on these I kind of like c for the references but it might indeed be a bit special while the attributes look more familiar. For the attributes, I think I would prefer b. Less verbose than a and equally descriptive. For the return either a or b seem fine. c is strange because you have a The last one ("for completeness") is indeed to confusing IMHO. Those variables are captured from the environment, not parameters to the closure. Maybe C++ lambda capture lists can also give some inspiration here but personally I think that syntax is too compressed and strange. |
to me the most natural is |
#[default_return]
|| Foo::new(1, 2, 3).to_string() |
Drawback of b is that the formatter wants two lines for one variable |
I feel like I prefer |
To me, it feels like, #[clone(weak(x), strong(self, rename_to = "y"), default_return(false))]
move |a, b, c| {
print!("{a} {b} {c} {x} {y}");
} While |
Unfortunately attributes on closures are unstable, so this is not really an option. Otherwise this would be a nice solution indeed. |
The problem with that is also that attribute macros only contain string values, not code. For the default return you'd ideally want something that is treated as code so it gets proper formatting and handling by rust-analyzer. |
So, after playing around with all the options this is what I'm going to implement: let x = clone!(
#[weak]
x,
#[strong(rename_to = "y")]
self,
#[default_return]
|| false,
move |a, b, c| {
print!("{a} {b} {c} {x} {y}");
}
); That seems to have the least amount of strangenesses syntax-wise and keeps strings to identifiers. |
why not |
for example from the top of my head: https://serde.rs/container-attrs.html |
As discussed on Matrix, this would have many drawbacks |
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
I wonder if the following would also work, since I think let x = clone!(
#[weak]
x,
#[strong]
self as y,
#[default_return]
false,
move |a, b, c| {
print!("{a} {b} {c} {x} {y}");
}
); |
That would also work but I thought it would be a bit strange because usually the right side of an |
I found it similar to the |
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
It's easy to change and I don't have a strong opinion either way. Anybody else? |
One problem that might happen is that currently you can do #[strong(rename_to = "x")]
1 + 2 + 3 With #[strong)]
1 + 2 + 3 as x the |
I think that also applies with |
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
One of the points here was for rustfmt and clippy to be able to do their work, using the |
Also please check the PR: #1424 Let's move discussion over there. |
As @pbor pointed out, |
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
This works correctly with rustfmt now, and thanks to using syn correctly also gives much better user experience with rust-analyzer. Also the `closure!` macro now supports plain weak references too. Fixes gtk-rs#1394
E.g. the following is treated correctly by rustfmt again because it's all valid Rust syntax. Using only valid Rust syntax would also allow us to make use of syn/quote, which hopefully makes it easier to preserve spans correctly, and make rust-analyzer and other tooling work better too.
The text was updated successfully, but these errors were encountered: