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

Block indentation in chains #1762

Closed
dragostis opened this issue Jul 1, 2017 · 3 comments · Fixed by #4246
Closed

Block indentation in chains #1762

dragostis opened this issue Jul 1, 2017 · 3 comments · Fixed by #4246
Labels
a-chains only-with-option requires a non-default option value to reproduce p-low poor-formatting

Comments

@dragostis
Copy link

dragostis commented Jul 1, 2017

Shouldn't chains with visual indentation indent blocks to the same visual indentation?

let a = b.iter()
         .map(|m| {
             m.hi()
         });

Instead of

let a = b.iter()
         .map(|m| {
             m.hi()
});
@nrc nrc added the only-with-option requires a non-default option value to reproduce label Jul 4, 2017
@dragostis
Copy link
Author

dragostis commented Nov 17, 2017

A simple example is:

let grammar: Vec<_> = ast.attrs.iter().filter(|attr| {
    match attr.value {
        MetaItem::NameValue(ref ident, _) => format!("{}", ident) == "grammar",
        _ => false
    }
}).collect();

which gets formatted to:

let grammar: Vec<_> = ast.attrs.iter()
                         .filter(|attr| match attr.value {
    MetaItem::NameValue(ref ident, _) => format!("{}", ident) == "grammar",
    _ => false
 })
                         .collect();

@dragostis
Copy link
Author

I'm interested to take a jab at this one if it doesn't require any major rearchitecturization.

@topecongiro
Copy link
Contributor

Thank you for being interested in contributing to rustfmt!

I do not think we need major reachitecturization to fix this. However, mixing chains and closures with visual indent has been known to be complex. It may be difficult to come up with an implementation which works in every case.

FWIW in rustfmt 0.3.6, the above example will be formatted to somewhat better code:

    let grammar: Vec<_> =
        ast.attrs.iter()
           .filter(|attr| match attr.value {
                       MetaItem::NameValue(ref ident, _) => format!("{}", ident) == "grammar",
                       _ => false,
                   })
           .collect();

ayazhafiz added a commit to ayazhafiz/rustfmt that referenced this issue Jun 8, 2020
Adds regression tests for the following issues which seem to be fixed on
master:

Closes rust-lang#1762
Closes rust-lang#2201
Closes rust-lang#2388
Closes rust-lang#2672
Closes rust-lang#2755
Closes rust-lang#2947
Closes rust-lang#2978
Closes rust-lang#3148
Closes rust-lang#3206

@topecongiro @calebcartwright appologies for the large number of issues
in this commit; if you prefer I can split it up into 2+.
ayazhafiz added a commit to ayazhafiz/rustfmt that referenced this issue Jun 9, 2020
Adds regression tests for the following issues which seem to be fixed on
master:

Closes rust-lang#1762
Closes rust-lang#2388
Closes rust-lang#2672
Closes rust-lang#2755
Closes rust-lang#2947
Closes rust-lang#2978
Closes rust-lang#3148
Closes rust-lang#3206

@topecongiro @calebcartwright appologies for the large number of issues
in this commit; if you prefer I can split it up into 2+.
calebcartwright pushed a commit that referenced this issue Jun 9, 2020
* Prune stale issues

Adds regression tests for the following issues which seem to be fixed on
master:

Closes #1762
Closes #2388
Closes #2672
Closes #2755
Closes #2947
Closes #2978
Closes #3148
Closes #3206

@topecongiro @calebcartwright appologies for the large number of issues
in this commit; if you prefer I can split it up into 2+.

* fixup! Prune stale issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-chains only-with-option requires a non-default option value to reproduce p-low poor-formatting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants