Skip to content

Confusing spread of width configurations #1984

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
alexheretic opened this issue Sep 19, 2017 · 7 comments
Closed

Confusing spread of width configurations #1984

alexheretic opened this issue Sep 19, 2017 · 7 comments

Comments

@alexheretic
Copy link
Member

It is great that rustfmt is configurable, however I found the amount of width configurations & defaults initially confusing. I expected the fmt rules to work on a single max width, so all wrapping logic would work against that.

I actually avoided using rustfmt for a while, as I mistakenly thought it couldn't handle one-line struct patterns etc. Now I realise it can, but has a separate width to keep to. Silly me. But perhaps this can be simpler?

After grepping through the docs I now have, for my personal projects

array_width = 100
chain_one_line_max = 100
comment_width = 100
max_width = 100
fn_call_width = 100
single_line_if_else_max_width = 100
struct_lit_width = 100
struct_variant_width = 100
# other rules

Which seems to work for now, so thank you. However, this also seems a little fragile & ugly. I'd really prefer to declare it once, perhaps having all rules match max_width unless explicitly configured otherwise.

Or some new rule:

max_width_that_applies_to_everything_and_causes_errors_if_you_try_to_use_the_other_ones = 100
@nrc
Copy link
Member

nrc commented Sep 20, 2017

cc #1974

I think whatever happens, we keep max_width

One approach for the others might be to have a single option - short_heuristic: bool which uses the default widths if true (default) and max_widthiffalse. If we want to be a little more fine-grained we could have short_expr_heuristicandshort_item_heursitic`.

@alexheretic
Copy link
Member Author

Yes I could deal with that. Although that name doesn't jump out at me as meaning what you describe. I do like a readable property name that I don't need to lookup.

max_width = 101
all_widths_use_max_width = true  # proposed

We already have fine grain control with all the other rules.

I would also suggest making sure the other rules end in width or are otherwise easily identifiable, and to reference all the rules in documentation of any of them (or just max_width). It makes sense that if I wanted to change one width, I'm probably interested in the others.

@nrc
Copy link
Member

nrc commented Sep 20, 2017

To clarify, I was proposing removing the existing options (other than max_width) .

HOw about short_width_heuristic?

@alexheretic
Copy link
Member Author

Ah right, you want to reduce the options in general.

Well what you suggest would satisfy my use case (unified width wrapping).

However, I would imagine that if one actually wanted separate widths for comments/if-else/structs etc (as you're proposing to keep default), they may also want to configure them.

A simple approach to reducing config would be just unify all widths and improve take_source_hints to allow manual wrapping for all applicable targets. But I imagine you guys have a reason for the separate widths, even if I don't quite see it.

Another idea, assuming the separate widths need to stay, would be to use factors of the max_width rather than absolute values. For example fn_call_width could be 0.6 instead of 60. This at least would scale with max_width, configurable or not.

@nrc nrc added this to the 1.0 milestone Nov 2, 2017
@nrc nrc closed this as completed in dd1fbca Nov 24, 2017
@ivanovaleksey
Copy link

ivanovaleksey commented Dec 20, 2017

Hello, with these change chain_width was removed.
This option allowed to set exact maximum width.
How is it intent to work now?
I can only enable/disable use_small_heuristics but maximum chain width would always be 60?

@nrc
Copy link
Member

nrc commented Dec 21, 2017

If you must have that level of configuration, you can build rustfmt yourself and change the values in width_heuristics. Longer term, it is possible we'll add different values as other options, but we'd need to see that the demand from users was there.

@PSeitz
Copy link
Contributor

PSeitz commented Dec 31, 2017

In my settings, there were basically 3 groups.

  • struct widths
  • fn_call_width
  • chain_width/single_line_if_else_max_width

struct_lit_width = 70
struct_variant_width = 70
chain_width = 100
single_line_if_else_max_width = 100
max_width = 155
fn_call_width = 155

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

No branches or pull requests

4 participants