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

"Fill match arms" assist removes comments #3687

Closed
Veetaha opened this issue Mar 23, 2020 · 2 comments · Fixed by #3761
Closed

"Fill match arms" assist removes comments #3687

Veetaha opened this issue Mar 23, 2020 · 2 comments · Fixed by #3761
Labels
E-has-instructions Issue has some instructions and pointers to code to get started E-medium good first issue

Comments

@Veetaha
Copy link
Contributor

Veetaha commented Mar 23, 2020

We need to preserve the useful trivia already located in the match body, otherwise, the assist totally erases it.
bug

Also, as a side note, it would be better if the match arms body were curly braces because I frequently find myself changing the parentheses to curlies in order to start writing the match arm body.

@matklad matklad added E-medium E-has-instructions Issue has some instructions and pointers to code to get started good first issue labels Mar 23, 2020
@matklad
Copy link
Member

matklad commented Mar 23, 2020

Impl notes: instead of replacing the whole MatchArmList, we should "append" new arms. This should works similar to ItemList::append_items.

That is, in that edit.rs file we should add

impl ast::MatchArmList {
    #[must_use]
    pub fn append_arms(&self, arms: impl IntoIterator<Item = ast::MatchArm>) -> ast::MatchArmList {

    }
}

@matklad
Copy link
Member

matklad commented Mar 23, 2020

(As a general design note, all assists should prefer to copy/move/append existing ranges of syntax nodes, written out by the user, rather than trying to construct fresh nodes)

bors bot added a commit that referenced this issue Mar 24, 2020
3700: fill match arms with empty block rather than unit tuple r=matklad a=JoshMcguigan

As requested by @Veetaha in #3689 and #3687, this modifies the fill match arms assist to create match arms as an empty block `{}` rather than a unit tuple `()`.

In one test I left one of the pre-existing match arms as a unit tuple, and added a body to another match arm, to demonstrate that the contents of existing match arms persist. 

Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
@bors bors bot closed this as completed in d2ea3f2 Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-has-instructions Issue has some instructions and pointers to code to get started E-medium good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants