Skip to content

Inconsistent formatting of attributes #2208

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
osa1 opened this issue Nov 29, 2017 · 7 comments
Closed

Inconsistent formatting of attributes #2208

osa1 opened this issue Nov 29, 2017 · 7 comments

Comments

@osa1
Copy link
Contributor

osa1 commented Nov 29, 2017

In this struct:

struct Test {
    /// foo
    #[serde(default)]
    pub join: Vec<String>,
    #[serde(default)] pub tls: bool,
}

Both fields have the same attribute but they're formatted differently, because of the comment line in the first one.

@nrc
Copy link
Member

nrc commented Nov 29, 2017

Hmm, I assume this isn't a problem with a non-doc comment? Doc comments are treated like attributes, so I think this is formatted like join has two attributes and thus they get stacked.

I think we might just want to change the default formatting for attributes on fields to be vertical rather than inline.

@osa1
Copy link
Contributor Author

osa1 commented Nov 29, 2017

I assume this isn't a problem with a non-doc comment?

Correct, if I make the comment line non-doc they're both formatted the same (attributes and fields on the same line).

@vishalsodani
Copy link
Contributor

@nrc I looked at this. Seems we just need to change a parameter to false in the config.rs.

same_line_attributes: bool, false, false

But many tests fail on this change which will have to be fixed.

@nrc
Copy link
Member

nrc commented Jan 4, 2018

Seems we just need to change a parameter to false in the config.rs.

That would change the default to always vertical attributes, yeah. I'm not 100% sure we want that, but I think we probably do. The test fallout is expected with any option default change.

If you wanted to take this on, I think it would be good to change the default and fix the tests and see what it looks like.

@vishalsodani
Copy link
Contributor

@nrc I have fixed all the tests. But there is one test related to #1046 which seems redundant now and could be removed.

@nrc
Copy link
Member

nrc commented Jan 9, 2018

@vishalsodani ah, hmm, perhaps this is more interesting to fix. Within an enum variant or tuple struct, then I think attributes should be on the same line, but on fields in a struct they should be on different lines. I think it is probably worth removing same_line_attributes altogether, find its uses and hard-wire true or false depending on whether it is a tuple field or a 'proper' field.

@RReverser
Copy link
Contributor

@nrc Now that same_line_attributes is gone, there seems to be no easy way to workaround #2389 (comment). More generally, may I ask why it can't be an option? Personally I prefer to always put attributes on its own line to separate it from an actual important item.

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

4 participants