-
Notifications
You must be signed in to change notification settings - Fork 1.8k
internal: Migrate assists to the structured snippet API, part 3 #15260
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
internal: Migrate assists to the structured snippet API, part 3 #15260
Conversation
`does_not_fill_wildcard_with_wildcard` and `does_not_fill_wildcard_with_partial_wildcard_and_wildcard` both made no modifications to the code, which is a problem for mutable ast porting as it generates a best-effort minimal set of text edits, and assists require at least one text edit.
Requires a hack in order to work inside of macros
Also tried to migrate |
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.
The migration itself looks great! Leaving a few small suggestions.
`clone_for_update` is relatively cheap in comparison, since making a node require parsing an entire source text Adds a test to make sure that it doesn't crash when multiple uses are present.
ca88d7a
to
a9889a0
Compare
Unfortunate thing to note is that currently rendering structured snippets into the real text form doesn't escape existing snippet bits so we're regressing to this behaviour of eating up backslashes 😔 I was eventually planning to change up the rendering anyway, but probably a good idea to focus on it now so that assist migration doesn't end up further regressing behaviour. |
After actually testing to see if the behaviour does regress for I did still end up making a branch that did change the rendering process, so that will still be helpful when user-provided code ends up in the tree diff. |
Thanks for the additional investigation (and the new PR ofc)! I believe this is good to go, but bors seems to be dormant now 🙃 @bors r+ |
☀️ Test successful - checks-actions |
…ykril internal: Migrate assists to the structured snippet API, part 4 Continuing from #15260 Migrates the following assists: - `add_turbo_fish` - `add_type_ascription` - `destructure_tuple_binding` - `destructure_tuple_binding_in_subpattern` I did this a while ago, but forgot to make a PR for the changes until now. 😅
Continuing from #15231
Migrates the following assists:
add_missing_match_arms
fix_visibility
promote_local_to_const
The
add_missing_match_arms
changes are best reviewed commit-by-commit since they're relatively big changes compared to the rest of the commits.