-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Span hygene and editor UX #2702
Conversation
Visit the preview URL for this PR (updated for commit fe8011a): https://yew-rs-api--pr2702-editor-spans-elxgb9lf.web.app (expires Tue, 31 May 2022 16:39:54 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Size Comparison
✅ None of the examples has changed their size significantly. |
Note, renaming inside prop expressions seem to be blocked by rust-lang/rust-analyzer#11272 in vscode. I don't know if other editors can handle it, would like to know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I haven't tested the changes but I don't see them breaking anything.
Also, can you create an issue in the Rust Analyzer repository asking how to test the changes in this PR? Rust Analyzer should also cover intellij-rust since they both have the same proc-macro expansion engine under-the-hood
Description
Fixes #2381
Additionally, renaming should now work correctly for usages of component types, properties and in expressions for keys and refs.
To expand on the latter: A quoted TokenStream can have two Spans attached to it:
resolved_at
andlocated_at
:located_at
resolved_at
. This not only impacts name resolution, but also renaming and other editor actions analyzing usage.So by decoupling the two and using
Span::call_site()
more often instead of using the same span for bothresolved_at
andlocated_at
, editors should be less confused which item should be targeted by a rename or usage lookup. Try it out in your editor. I don't have a good approach to automating these UX tests… Sample manual test code:Checklist