Skip to content

Inconsistent indentation of #[rustfmt::skip] #4699

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

Closed
Maxidejf opened this issue Feb 15, 2021 · 3 comments
Closed

Inconsistent indentation of #[rustfmt::skip] #4699

Maxidejf opened this issue Feb 15, 2021 · 3 comments
Assignees

Comments

@Maxidejf
Copy link

I ended up using a lot of #[rustfmt::skip] attributes in my code to preserve our coding standards until rust fmt (hopefully) evolves its output config options enough so that they can at some point be deleted.
However, Rust fmt does not always indent the skip attributes to where I'd expect.

This is a (somewhat) minimal example of what I mean:

Original Input

pub enum CalibrationType {
    AlphaBeta,
    BinShift,
    Enum,
    None,
}

impl CalibrationType {
    #[rustfmt::skip] // In this case aligning the match branches makes for more readable code.
    pub fn parse(value: &str) -> Option<Self> {
        match value {
            "alpha-beta" => Some(Self::AlphaBeta),
            "bin-shift"  => Some(Self::BinShift),
            "enum"       => Some(Self::Enum),
            "none"       => Some(Self::None),
            _            => None
        }
    }
    
    #[rustfmt::skip] // In this case aligning the match branches makes for more readable code.
    pub fn to_str(&self) -> &'static str {
        match self {
            Self::AlphaBeta => "alpha-beta",
            Self::BinShift  => "bin-shift",
            Self::Enum      => "enum",
            Self::None      => "none"
        }
    }
}

Suggested Output
While checking what the changes would be via cargo +nightly fmt -- --check the suggested output looks like this (second skip attribute loses its indent):

pub enum CalibrationType {
    AlphaBeta,
    BinShift,
    Enum,
    None,
}

impl CalibrationType {
    #[rustfmt::skip] // In this case aligning the match branches makes for more readable code.
    pub fn parse(value: &str) -> Option<Self> {
        match value {
            "alpha-beta" => Some(Self::AlphaBeta),
            "bin-shift"  => Some(Self::BinShift),
            "enum"       => Some(Self::Enum),
            "none"       => Some(Self::None),
            _            => None
        }
    }
    
#[rustfmt::skip] // In this case aligning the match branches makes for more readable code.
    pub fn to_str(&self) -> &'static str {
        match self {
            Self::AlphaBeta => "alpha-beta",
            Self::BinShift  => "bin-shift",
            Self::Enum      => "enum",
            Self::None      => "none"
        }
    }
}

It seems that unless the skip attribute is on the first line within the impl block, the indentation is lost. The indentation seems to work fine within the functions.

Meta

  • rustfmt version: rustfmt 1.4.20-stable (48f6c32e 2020-08-09)
  • From where did you install rustfmt?: rustup component add rustfmt --toolchain nightly
@calebcartwright
Copy link
Member

Thank you for reaching out @Maxidejf! I am a little confused by the divergence of your issue from the issue template. The issue template is structured so that users can report what their original input was, what the rustfmt-produced output of that input was, and then third is what the user thinks the expected formatting should be.

I'm currently interpreting your issue description to mean that rustfmt is producing the output under your "Suggested output" section, though if you could confirm/clarify that'd be helpful.

Additionally, this looks a bit like a duplicate of #4398 and I'm unable to reproduce this on more recent versions of rustfmt (1.4.36 on nightly). Are you able to reproduce on more recent versions?

@Maxidejf
Copy link
Author

My apologies from diverging from the template (yes, you understood it correctly).

I did update to rustfmt 1.4.36-nightly (7de6968e 2021-02-07) and indeed it went away. I'm sorry, I tried to search for similar existing issues before posting but I've missed #4398. I'm closing the issue. Thank you! :)

@calebcartwright
Copy link
Member

No worries! Glad it's working for you now

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

3 participants