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.

Sign up for GitHub

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

Jump to bottom

PreferSameLine brace_style with trailing_comma Vertical produces weird results #2321

Open
djc opened this issue Dec 30, 2017 · 3 comments · May be fixed by #5556 or #5895
Open

PreferSameLine brace_style with trailing_comma Vertical produces weird results #2321

djc opened this issue Dec 30, 2017 · 3 comments · May be fixed by #5556 or #5895
Assignees
@gentoid
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors only-with-option requires a non-default option value to reproduce poor-formatting

Comments

@djc
Copy link
Contributor

djc commented Dec 30, 2017 •
edited by rustbot
Loading

Upgrading rustfmt-nightly from 0.2.15 to 0.3.4 created this change:

@@ -104,8  122,7 @@ impl FetchCommandMessages {
 
 pub trait FetchBuilderMessages
 where
-    Self: Sized,
-{
     Self: Sized, {
     fn prepare(self) -> FetchCommandMessages;
 
     fn num(self, num: u32) -> FetchCommandMessages {

This seems like a bug, since trailing_comma = "Vertical" should not trigger if the opening brace is on the same line.
@topecongiro topecongiro added only-with-option requires a non-default option value to reproduce poor-formatting labels Dec 30, 2017
@ytmimi
Copy link
Contributor

ytmimi commented Jul 19, 2022

Confirming this is reproducible with rustfmt 1.5.1-nightly (0cb294f0 2022-07-12) and configuration options trailing_comma = "Vertical" and brace_style=PreferSameLine.

Input

pub trait FetchBuilderMessages
where
    Self: Sized,
{
     fn prepare(self) -> FetchCommandMessages;
}

output

pub trait FetchBuilderMessages
where
    Self: Sized, { // <--- Note trailing comma is not removed from the bound
    fn prepare(self) -> FetchCommandMessages;
}

If someone is interested in taking this on, I'd start by checking out these lines inrewrite_bounds_on_where_clause:

rustfmt/src/items.rs

Lines 2872 to 2876 in a7bf009

let comma_tactic = if where_clause_option.suppress_comma || force_single_line {
SeparatorTactic::Never
} else {
context.config.trailing_comma()
};

@ytmimi ytmimi added the good first issue Issues up for grabs, also good candidates for new rustfmt contributors label Jul 19, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

Linking tracking issues for brace_style(#3376) and trailing_comma (#3379)

@hirasawayuki hirasawayuki linked a pull request Oct 2, 2022 that will close this issue
Open
@gentoid
Copy link
Contributor

gentoid commented Aug 27, 2023

@rustbot claim

@gentoid gentoid linked a pull request Aug 27, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gentoid gentoid

Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors only-with-option requires a non-default option value to reproduce poor-formatting
Projects
None yet
Milestone
No milestone
4 participants
@djc @gentoid @topecongiro @ytmimi