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

Add postfix completion for let else #15730

Merged
merged 3 commits into from Jan 29, 2024
Merged

Conversation

Alainx277
Copy link
Contributor

Adds a postfix completion for let else syntax, similar to the if let postfix.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2023
@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2023
@Alainx277 Alainx277 requested a review from Veykril October 9, 2023 17:14
@Alainx277
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 13, 2023
Copy link
Contributor

@saiintbrisson saiintbrisson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nice PR and auto completions. What do you think of adding the postfix for non-try types as well?

@Alainx277
Copy link
Contributor Author

Alainx277 commented Nov 19, 2023

Super nice PR and auto completions. What do you think of adding the postfix for non-try types as well?

Thanks. I think it could be useful, but it also doesn't exist for if let yet. Maybe you could make a follow-up PR that adds it for both? It would be nice if the enum variants were placeholders (like "continue, break, return" in this PR) 👍

@Alainx277
Copy link
Contributor Author

@Veykril sorry for the ping, but I was wondering if this MR will get another review? I've addressed the last review over 2 months ago and would love for this small improvement to make it into RA.

@Veykril
Copy link
Member

Veykril commented Dec 21, 2023

Apologies I just don't have time on my hands to go through the PR backlog until january.

Comment on lines 95 to 99
&if in_loop {
format!("let Ok($1) = {receiver_text} else {{\n ${{2|continue,break,return|}};\n}};\n$0")
} else {
format!("let Ok($1) = {receiver_text} else {{\n ${{2:return}};\n}};\n$0")
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
&if in_loop {
format!("let Ok($1) = {receiver_text} else {{\n ${{2|continue,break,return|}};\n}};\n$0")
} else {
format!("let Ok($1) = {receiver_text} else {{\n ${{2:return}};\n}};\n$0")
},
&format!("let Ok($1) = {receiver_text} else {{\n $0\n}};"),

Let's not prefill these, there are more options people could strife for. Same for the option one below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit sad to lose the convenience, but I see how these could be annoying when you want to do more complex things.

@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 2, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jan 27, 2024

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

@Alainx277
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 27, 2024
@Veykril
Copy link
Member

Veykril commented Jan 29, 2024

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jan 29, 2024

📌 Commit 7ec32d0 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jan 29, 2024

⌛ Testing commit 7ec32d0 with merge d13951f...

@bors
Copy link
Collaborator

bors commented Jan 29, 2024

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

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