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

rustfmt.toml for smoltcp #56

Closed
podhrmic opened this issue Oct 13, 2017 · 7 comments
Closed

rustfmt.toml for smoltcp #56

podhrmic opened this issue Oct 13, 2017 · 7 comments

Comments

@podhrmic
Copy link
Contributor

Would be possible to add a rustfmt.toml file (see https://github.com/rust-lang-nursery/rustfmt/blob/master/Configurations.md ) specifying the preferred formatting for smoltcp?

That way it would be easy to keep consistent formatting.

For example I noticed that the default rustfmt settings are different than what smoltcp is using, so it is hard to automatically format existing files.

@whitequark
Copy link
Contributor

Sure. Do you think you could play with rustfmt's options to get an output very close to the current one? Then I'll merge that and reformat the rest.

@podhrmic
Copy link
Contributor Author

I ll give it a try. I think the main difference is the alignment - i.e. this is what is in smoltcp now:

    /// Return the header length.
    /// The result depends on the value of the message type field.
    pub fn header_len(&self) -> usize {
        match self.msg_type() {
            Message::EchoRequest    => field::ECHO_SEQNO.end,
            Message::EchoReply      => field::ECHO_SEQNO.end,
            Message::DstUnreachable => field::UNUSED.end,
            _ => field::CHECKSUM.end // make a conservative assumption
        }
    }

And this is after rustfmt with default settings:

    /// Return the header length.
    /// The result depends on the value of the message type field.
    pub fn header_len(&self) -> usize {
        match self.msg_type() {
            Message::EchoRequest => field::ECHO_SEQNO.end,
            Message::EchoReply => field::ECHO_SEQNO.end,
            Message::DstUnreachable => field::UNUSED.end,
            _ => field::CHECKSUM.end, // make a conservative assumption
        }
    }

So I am not sure how much you care about the right alignment?

@whitequark
Copy link
Contributor

To be honest I find that it helps readability a lot.

@podhrmic
Copy link
Contributor Author

After poking around it looks like there is no setting for rustfmt that would keep the LHS => RHS alignment as you have it. Apparently it is a discouraged style for Rust.

So the question is - do you prefer to keep your current formatting even though it means no automatic format checking, or do you want to switch to the format mentioned above?

@whitequark
Copy link
Contributor

Let's keep it, but just so that your work doesn't go nowhere, please post your rustfmt config here. If we ever switch to using rustfmt I'll make use of it.

@podhrmic
Copy link
Contributor Author

OK. Place rustfmt.toml in the root dir of the project.

The contents are simple and can be further tuned:

attributes_on_same_line_as_field = false
attributes_on_same_line_as_variant = false
struct_field_align_threshold = 20
tab_spaces = 4

See https://github.com/rust-lang-nursery/rustfmt for details

Sorry, something went wrong.

@whitequark
Copy link
Contributor

I looked at the diff produced by rustfmt. It is 11937 line long and readability suffers, particularly badly with tests. Thank you for your work but I don't think I'm going to use rustfmt for my projects.

Sorry, something went wrong.

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

No branches or pull requests

2 participants