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

Multi-line expressions on both sides of an assignment shouldn't wrap to the same indent level #189

Open
joshtriplett opened this issue Dec 28, 2023 · 5 comments · Fixed by rust-lang/rust#119838

Comments

@joshtriplett
Copy link
Member

I discovered this issue in some code I'm working on, and constructed this sample to match the issue I observed:

impl SomeType {
    fn method(&mut self) {
        self.array[array_index as usize]
            .as_mut()
            .expect("thing must exist")
            .extra_info = Some(ExtraInfo {
            parent,
            count: count as u16,
            children: children.into_boxed_slice(),
        });
    }
}

The fields of ExtraInfo should not be wrapping to the same indent level as the method chain. I can imagine a few ways to format this better, and don't feel strongly about which one, just that this definitely isn't the right formatting.

@joshtriplett
Copy link
Member Author

This also happens with a simpler version:

impl SomeType {
    fn method(&mut self) {
        self.array[array_index as usize]
            .as_mut()
            .expect("thing must exist")
            .extra_info =
            long_long_long_long_long_long_long_long_long_long_long_long_long_long_long;
    }
}

@joshtriplett
Copy link
Member Author

Turns out this happens with other binary operators like + too, not just =.

@compiler-errors came up with a concise rule for how to fix these cases:

The parent identation that we use to make any relative indent/deindent for binary operators should use the indentation level of the last line of the LHS, not the first.

@traviscross
Copy link

We discussed this in the T-style meeting today, and the consensus was that we should adopt the rule that Josh just mentioned:

The parent indentation that we use to make any relative indent/deindent for binary operators should use the indentation level of the last line of the LHS, not the first.

(Credit to @compiler-errors for this rule.)

joshtriplett added a commit to joshtriplett/rust that referenced this issue Jan 11, 2024
Use the indentation of the *last* line of the first operand, not the first.

Fixes rust-lang/style-team#189
joshtriplett added a commit to joshtriplett/rust that referenced this issue May 14, 2024
Use the indentation of the *last* line of the first operand, not the first.

Fixes rust-lang/style-team#189
compiler-errors added a commit to compiler-errors/rust that referenced this issue May 14, 2024
…ent, r=compiler-errors

style-guide: When breaking binops handle multi-line first operand better

Use the indentation of the *last* line of the first operand, not the first.

Fixes rust-lang/style-team#189
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 14, 2024
Rollup merge of rust-lang#119838 - joshtriplett:style-guide-binop-indent, r=compiler-errors

style-guide: When breaking binops handle multi-line first operand better

Use the indentation of the *last* line of the first operand, not the first.

Fixes rust-lang/style-team#189
@johnhuichen
Copy link

@rustbot claim

@rustbot
Copy link
Collaborator

rustbot commented Aug 31, 2024

Error: Malformed triagebot.toml in default branch.
TOML parse error at line 5, column 7
|
5 | "*" = "style"
| ^^^^^^^
invalid type: string "style", expected a sequence

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

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 a pull request may close this issue.

4 participants