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

Clarify muliti-line chain elements #163

Closed
wants to merge 1 commit into from

Conversation

camsteffen
Copy link

Say (and show) that a multi-line element causes all surrounding elements to be on their own line, not just later elements. But also maintain the caveat where multiple elements may be on the first line.

I added a closure the example so that it is more certainly a multi-line element.

See #151

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Thank you for opening a PR with suggested changes, and apologies for the late review.

Unfortunately, however, this isn't workable in its current form both because it's an incorrect description of the current formatting rules and because it introduces a new subjective term "surrounding" that'd be a step back from the plan to improve clarity.

As discussed in #151 the presence a multiline element does implact the chain element(s) that precede it, only those that follow. You can of course suggest making a major change to the chain formatting rules if you'd like them to be different, but in the context of updating the wording to provide clarity of the current rules we can't change the rule itself.

Here's an example that provides a good illustration of why the rules are what they currently are and why we wouldn't the preceding b element to have to be block indented and on its own line just because it's followed by foo

fn foo() {
    a.b.foo(|| {
        expr;
    })
    .bar
    .baz
}

Doing so would directly contradict the other required rules discussed in #151 which would not work. I'm definitely still open to expanding the wording if you think there's an opportunity to clarify the current ruling, but cannot move to the proposed wording due to the aforementioned issues.

@camsteffen
Copy link
Author

Thank you for the review. I finally understand what was wrong with my understanding regarding "surrounding elements". I updated the PR with another attempt to clarify, hopefully without changing the rules this time.

@calebcartwright
Copy link
Member

Closing this as the guide doesn't live here anymore (it's in r-l/rust), discussion remains tracked in #151 in the event there are any additional examples or clarifications (including specific ones proposed in this PR) that we'd like to cherry pick over to the new guide location

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants