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

feat: On type format '(', by adding closing ')' automatically #15532

Merged

Conversation

SomeoneToIgnore
Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore commented Aug 29, 2023

If I understand right, () can surround pretty much the same {} can, so add another on type formatting pair for convenience: sometimes it's not that pleasant to write parenthesis in Some(2).map(|i| (i, i+1)) cases and I would prefer r-a to do that for me.

One note: currently,

stdx::never!(snippet_edit.is_none(), "on type formatting shouldn't use structured snippets");
fires always.
Should we remove the assertion entirely now, since apparently things work in release despite that check?

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2023
@DropDemBits
Copy link
Contributor

Should we remove the assertion entirely now, since apparently things work in release despite that check?

Ah, I was the one who added that assert in, and only realized now that this should be a stdx::always instead of a stdx::never 😅

More context on this assert I originally added this assert since structured snippets add in placeholders & tabstops later on instead of embedding them directly, and I didn't think it was worth it right now to port the on-type formatting to structured snippets as it only uses one tabstop in a fixed location.

@SomeoneToIgnore
Copy link
Contributor Author

Thank you, works much better with the different assert 🙂

I've pushed the commit here, but feel free to create a separate PR, you have better context and can describe it better.

@bors
Copy link
Collaborator

bors commented Sep 1, 2023

☔ The latest upstream changes (presumably #15542) made this pull request unmergeable. Please resolve the merge conflicts.

@Veykril
Copy link
Member

Veykril commented Sep 6, 2023

Now I wonder, we might as well add [ to the mix here no?

@Veykril
Copy link
Member

Veykril commented Sep 6, 2023

Well we can do that in separate PR, but I think that should work in all positions there likewise.
@bors r+

@bors
Copy link
Collaborator

bors commented Sep 6, 2023

📌 Commit da78617 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 6, 2023

⌛ Testing commit da78617 with merge 77b359a...

@SomeoneToIgnore
Copy link
Contributor Author

Now I wonder, we might as well add [ to the mix here no?

I personally rarely index something nested with this, so was afraid to add it myself.
Also, was not sure if it's the same as ( rule-wise to apply its counterpart?
Anyway, not sure if very requested feature, so I restrained from bloating the PR.

Ideally, I'd add | since typing closures is sometimes hard, but that needs a but different set of rules.

@Veykril
Copy link
Member

Veykril commented Sep 6, 2023

I was more thinking of arrays than indexing here, unsure whether its useful though.

@bors
Copy link
Collaborator

bors commented Sep 6, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 77b359a to master...

@bors bors merged commit 77b359a into rust-lang:master Sep 6, 2023
10 checks passed
@SomeoneToIgnore SomeoneToIgnore deleted the more-brackets-on-type-formatting branch September 6, 2023 20:52
@lnicola lnicola changed the title On type format '(', by adding closing ')' automatically feat: On type format '(', by adding closing ')' automatically Sep 9, 2023
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.

None yet

5 participants