Skip to content

internal: Migrate assists to the structured snippet API, part 4 #15874

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

Merged
merged 10 commits into from
Nov 15, 2023

Conversation

DropDemBits
Copy link
Contributor

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. 😅

Mirrors `PathSegment's` version, except that it always generates a
turbofish
`add_type_ascription` is still left as-is since it's a different assist
Way for setting and removing the type ascription of a let stmt
Due to the way the current tree mutation api works, we need to collect
changes before we can apply them to the real syntax tree, and also can only
switch to a file once.

`destructure_tuple_binding_in_sub_pattern` also gets migrated even
though can't be used.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 12, 2023
Needed so that the `tuple_pat` node gets added to the syntax tree,
which is required as we're using structured snippets.
@DropDemBits DropDemBits force-pushed the structured-snippet-migrate-4 branch from d477979 to c32ed45 Compare November 14, 2023 22:50
Assist wasn't applicable when the let statement was missing a pattern
before, so we should do the same now.
@DropDemBits DropDemBits force-pushed the structured-snippet-migrate-4 branch from c32ed45 to 3f99a56 Compare November 14, 2023 23:44
@Veykril
Copy link
Member

Veykril commented Nov 15, 2023

Right, we were actively moving assists to use the mutable syntax node API which is at odds with my opinion of them nowadays #15710 😓 Wonder if we can figure out a solution for this API should we ever move away from them.
But that aside, moving this is fine for now
@bors r+

@bors
Copy link
Contributor

bors commented Nov 15, 2023

📌 Commit 3f99a56 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 15, 2023

⌛ Testing commit 3f99a56 with merge 535eb0d...

@bors
Copy link
Contributor

bors commented Nov 15, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 535eb0d to master...

@bors bors merged commit 535eb0d into rust-lang:master Nov 15, 2023
@DropDemBits
Copy link
Contributor Author

@Veykril

Right, we were actively moving assists to use the mutable syntax node API which is at odds with my opinion of them nowadays #15710 😓

Haven't seen that issue until now (slowly getting back into contributing in general after a break), but hard agree on on the problems with the mutable syntax tree api. It is a step up from the text-based apis, but still not ideal 😔

Once I'm done with the structured snippet port + a few structured snippet related odds & ends (according to my personal todo list, maybe mid-December?), I do have an interest in tackling things like the syntax quasi-quote api and at the very least an auto-indenter (though we'll see how I feel by then).

Wonder if we can figure out a solution for this API should we ever move away from them.

The neat thing about how structured snippets are implemented is that SourceChangeBuilder only cares about where nodes & tokens are, and then once the edit is finished, the snippet locations are converted into the corresponding TextRange & TextSize for the protocol conversion layer to handle, i.e. it does not depend on the mutable syntax tree api 😁

In fact, I think it'd be even better with a different node tracking api, as I've run into the issue of "losing" syntax nodes quite a few times (e.g. for add_derive, and in the future of supporting placeholder groups for extract_variable).

@DropDemBits DropDemBits deleted the structured-snippet-migrate-4 branch November 15, 2023 12:49
@Veykril
Copy link
Member

Veykril commented Nov 15, 2023

A related issue regarding node tracking #9649

bors added a commit that referenced this pull request Jan 2, 2024
…ykril

internal: Migrate assists to the structured snippet API, part 5

Continuing from #15874

Migrates the following assists:

- `extract_variable`
- `generate_function`
- `replace_is_some_with_if_let_some`
- `replace_is_ok_with_if_let_ok`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants