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

Vertical formatting of literal vec and function arg style macros #5190

Closed
OJFord opened this issue Jan 24, 2022 · 5 comments
Closed

Vertical formatting of literal vec and function arg style macros #5190

OJFord opened this issue Jan 24, 2022 · 5 comments
Labels

Comments

@OJFord
Copy link

OJFord commented Jan 24, 2022

I can't find a way to influence the formatting of:

pub(crate) static ref ROUTES: Vec<rocket::Route> = routes![a, b, c];

except to make it long enough that max_width (or make that small enough such that it) comes into effect.

Even then, the result is:

pub(crate) static ref ROUTES: Vec<rocket::Route> = 
    routes![a, b, c];

and eventually:

pub(crate) static ref ROUTES: Vec<rocket::Route> = routes![
   a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c,
   a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c, a, b, c,
];

whereas I'd like to achieve:

pub(crate) static ref ROUTES: Vec<rocket::Route> = routes![
    a,
    b,
    c,
];

In general, I think macro![] should follow any current and future array rules, and likewise macro!() should follow those for function invocations.

Ideally then, for the 'vertical' formatting case, I'd be able to set something like max_single_line_elements = 0 (which to be clear is made up, and yes I would set it to 0 - I'd rather a diff adding the first element was only adding an element, not also /- the declaration of variable and type etc.) rather than anything width-based.

@calebcartwright
Copy link
Member

Thanks for posting! To be honest something feels off on the default behavior, but I'd need to dig back through some old RFCs to check. Few specific responses:

and eventually:

While your example holds true, I want to note that you're missing the comma delimiters between the final a/b pair on each line which in turn results in rustfmt not formatting the macro call whatsoever. Adding those allows it to format, but does still do the same compressed-style layout you were trying to demonstrate

In general, I think macro![] should follow any current and future array rules, and likewise macro!() should follow those for function invocations.

rustfmt does not operate on its own opinions (nor ours either!), but instead is a tooling implementation for the Rust Style Guide which provides various prescriptions for each syntactical construct, e.g. https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/expressions.md#array-literals.

AFAIK the only way to force the pure vertical formatting would be to make one of those elements several characters longer, and this is the suspect-looking part I alluded to earlier.

Ideally then, for the 'vertical' formatting case, I'd be able to set something like max_single_line_elements = 0 (which to be clear is made up, and yes I would set it to 0 - I'd rather a diff adding the first element was only adding an element, not also /- the declaration of variable and type etc.) rather than anything width-based.

This ties back to the more general request for more control over layout in calls/call-like constructs (refs #2010/#4146). I appreciate the proposal of a solution, but I don't envision it being practical to go that route. I'd rather we stick with the standard layout options for calls like we provide for params via fn_params_layout, and then investigate a Preserve style variant like we're doing with chain items. I'm not explicitly opposed to eventually supporting some form of variant that incorporates a max, but don't expect we'll attempt to tackle that ourselves as history has shown that trying to combine the standard AST-driven, rule-based steamroll formatting with partial preservation based on input is a recipe for idempotency issues.

@OJFord
Copy link
Author

OJFord commented Jan 27, 2022

Thanks for your response.

While your example holds true, I want to note that you're missing the comma delimiters between the final a/b pair on each line which in turn results in rustfmt not formatting the macro call whatsoever.

Oops, that's what I get for making something up here immaterially different (but different) from what I actually ran - or so I thought!

That was just a typo, I'll edit it in to avoid others picking up on it, just mentioning it so as not to make your comment on it confusing.

investigate a Preserve style variant like we're doing with chain items.

I wasn't aware of that, but that's something I almost mentioned by comparison to black for Python - it will maintain a trailing comma, and if present (even if currently single line) it forces multiline 'vertical' formatting.

That is:

routes: List[rocket.Route] = [a, b, c,]

Becomes:

routes: List[rocket.Route] = [
    a,
    b,
    c,
]

@calebcartwright
Copy link
Member

I wasn't aware of that, but that's something I almost mentioned by comparison to black for Python - it will maintain a trailing comma, and if present (even if currently single line) it forces multiline 'vertical' formatting.

Please let me know if there's some obvious dots I'm just failing to connect, but this seems like a different topic no?

We have the trailing_comma config option that allows users to control this. However, it's a different story with macro calls because the insertion/removal of any tokens can be problematic in the general sense (as we've no idea whether that could impact a given macro), and there's actually a known, but unrelated, bug on comma handling in some of those macro cases

@calebcartwright
Copy link
Member

Going to close this out as I was remembered that whether or not the formatting goes vertically is determined by the length of the elements in the array and whether or not they are all "short'. That threshold for what qualifies as "short" is currently a hardcoded internal option.

However, we're going to open that up on the public config surface (refs #5228) to allow users to control this aspect of array formatting where you'd be able to do something like whatever_we_name_the_new_option = 0 if you want to force the vertical formatting on the example in the description.

@OJFord
Copy link
Author

OJFord commented Mar 5, 2022

Nice, thank you!

Sorry I didn't reply to your previous post. Just quickly - the link to me was that in black the trailing comma => multiline vertical formatting. In rustfmt, it is indeed a different topic, because it'll add/remove/not-change a trailing comma without implication for the horizantal vs. vertical formatting.

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

No branches or pull requests

3 participants