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

Leave whitespace for logically grouped comments alone #1283

Open
ppannuto opened this issue Jan 22, 2017 · 5 comments
Open

Leave whitespace for logically grouped comments alone #1283

ppannuto opened this issue Jan 22, 2017 · 5 comments

Comments

@ppannuto
Copy link

Is seems that rustfmt is identifies an ending | (or perhaps simply not ;) as a continued block and doesn't mess with the whitespace, but it will change the formatting of the last entry:

Before

fn reduced_example(scaler: usize) {
    let control = (1 << 16) |     // Clock enable
                  (scaler << 8) | // Set PSEL to based on period
                  (1 << 7)  |     // Flash calibration done (set to default)
                  (1 << 1)  |     // Disable after reset
                  (1 << 0);       // Enable
}

After

fn reduced_example(scaler: usize) {
    let control = (1 << 16) |     // Clock enable
                  (scaler << 8) | // Set PSEL to based on period
                  (1 << 7)  |     // Flash calibration done (set to default)
                  (1 << 1)  |     // Disable after reset
                  (1 << 0); // Enable
}

I believe this is a similar but distinct case from #1275

@ppannuto
Copy link
Author

ppannuto commented Jan 22, 2017

Another similar example:

Before

    PA[08].configure(Some(A)); // FTDI_RTS - USART0 RTS
    PA[09].configure(None);    // ACC_INT1 - FXOS8700CQ Interrupt 1
    PA[10].configure(None);    // unused
    PA[11].configure(Some(A)); // FTDI_OUT - USART0 RX FTDI->SAM4L
    PA[12].configure(Some(A)); // FTDI_IN - USART0 TX SAM4L->FTDI
    PA[13].configure(None);    // RED_LED
    PA[14].configure(None);    // BLUE_LED
    PA[15].configure(None);    // GREEN_LED

After

    PA[08].configure(Some(A)); // FTDI_RTS - USART0 RTS
    PA[09].configure(None); // ACC_INT1 - FXOS8700CQ Interrupt 1
    PA[10].configure(None); // unused
    PA[11].configure(Some(A)); // FTDI_OUT - USART0 RX FTDI->SAM4L
    PA[12].configure(Some(A)); // FTDI_IN - USART0 TX SAM4L->FTDI
    PA[13].configure(None); // RED_LED
    PA[14].configure(None); // BLUE_LED
    PA[15].configure(None); // GREEN_LED

I think the general rule here is that vertically aligned // comment should not have leading whitespace removed.

rustfmt v0.6.3 used to leave these alone, v0.7.1 now corrupts them

@nrc nrc added the a-comments label Jan 23, 2017
@nrc
Copy link
Member

nrc commented Jan 23, 2017

Rustfmt is not smart enough to maintain alignment like this, I think what happened is that it always tried to remove the whitespace, but for some reason did not manage to do so in some cases, and that set of cases has changed.

@topecongiro
Copy link
Contributor

Rustfmt is not smart enough to maintain alignment like this, I think what happened is that it always tried to remove the whitespace, but for some reason did not manage to do so in some cases, and that set of cases has changed.

When there are comments in between binary expressions or chain, rustfmt gives up and just copy & paste the original snippet. That's why the comments inside binary operations are kept aligned, while the spaces between ; and the last line comment is properly removed into a single space.

@nrc nrc added the p-low label Jun 24, 2018
@topecongiro topecongiro added this to the 3.0.0 milestone Jun 7, 2020
@teohhanhui
Copy link

In my case I actually want the opposite behaviour, e.g. to stop this:

    let res = pipe!(
        2, // 2
        |x| x * 10, // 20
        |x| x - 3, // 17
        |x| x   5, // 22
    );

from becoming this:

    let res = pipe!(
        2,          // 2
        |x| x * 10, // 20
        |x| x - 3,  // 17
        |x| x   5,  // 22
    );

@calebcartwright
Copy link
Member

In my case I actually want the opposite behaviour, e.g. to stop this:

    let res = pipe!(
        2, // 2
        |x| x * 10, // 20
        |x| x - 3, // 17
        |x| x   5, // 22
    );

from becoming this:

    let res = pipe!(
        2,          // 2
        |x| x * 10, // 20
        |x| x - 3,  // 17
        |x| x   5,  // 22
    );

@teohhanhui - Realize it's a similar theme, but what you're talking about is actually unrelated and a duplicate of #4108

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

No branches or pull requests

5 participants