Audit config options

[nrc]

Are there options which we should not bother to support? Options which could be combined? Of those we should keep, which are certain enough of (both that they should exist and of their behaviour) that we should consider them stable?

[topecongiro]

A list of config options, in a bit of organized fashion (ran out of gas).

Catch-all

• control_style -> indent_style
• where_style -> indent_style

Brace

I think we can combine these into a single brace_style.

• control_brace_style
• fn_brace_style -> brace_style
• item_brace_style -> brace_style

Comment

• normalize_comments
• wrap_comments

Indent

I think we should make these names consistent (.._indent).
Also I am wondering if we can combine them into more 'general' config option, like expr_indent or item_indent. We do not want to combine block-style indent and visual-style indent together, so having separate config options for each ExprKind seems unnecessary IMO.

• array_layout -> array_indent -> indent_style
• chain_indent -> indent_style
• fn_args_layout -> fn_args_indent -> indent_style
• fn_call_style -> fn_call_indent -> indent_style
• generics_indent -> indent_style
• imports_indent
• struct_lit_style -> struct_lit_indent -> indent_style

Match arm

• indent_match_arms
• match_block_trailing_comma
• match_pattern_separator_break_point
• wrap_match_arms

Space

It feels to me that these are too narrow to support except space_around_ranges.

• space_after_bound_colon
• space_after_struct_lit_field_colon
• space_after_type_annotation_colon
• spaces_around_ranges
• space_before_bound
• space_before_struct_lit_field_colon
• space_before_type_annotation
• spaces_within_angle_brackets
• spaces_within_parens
• spaces_within_square_brackets
• type_punctuation_density

String

• format_strings
• force_format_strings

Vertical alignement

• struct_field_align_threshold

Width

Again, we should name these consistently (chain_one_line_max to chain_width and tab_spaces to tab_width).

• array_width
• chain_one_line_max -> chain_width
• comment_width
• fn_call_width
• max_width
• single_line_if_else_max_width
• struct_lit_width
• struct_variant_width
• tab_spaces

Horizontal v.s. Vertical v.s. Mixed

• array_horizontal_layout_threshold
• fn_args_density
• imports_layout
• where_density

Sort items

• reorder_extern_crates
• reorder_extern_crates_in_group
• reorder_imported_names
• reorder_imports
• reorder_imports_group

What to do with empty block body

We can combine these into empty_block_body_single_line.

• fn_empty_single_line
• impl_empty_single_line

Allow single line

• fn_single_line
• struct_lit_multiline_style -> struct_lit_single_line

Legacy

These options do not take effect or work well with the RFC style. I think it is safe to deprecate these options.

• closure_block_indent_threshold
• fn_return_indent
• fn_args_paren_newline
• take_source_hints
• where_layout
• where_pred_indent

Not exposed to users

• file_lines
• verbose

Others

report_todo and report_fixme are nice to have, but not a typical job for a format tool IMO.

• attributes_on_same_line_as_field
• attributes_on_same_line_as_variant
• binop_separator
• chain_split_single_child
• combine_control_expr
• condense_wildcard_suffixes
• disable_all_formatting
• error_on_line_overflow
• error_on_line_overflow_comments
• force_explicit_abi
• hard_tabs
• merge_derives
• multiline_closure_forces_block
• multiline_match_arm_forces_block
• newline_style
• remove_blank_lines_at_start_or_end_of_block
• report_fixme
• report_todo
• required_version
• skip_children
• trailing_comma
• trailing_semicolon
• use_try_shorthand
• write_mode

[nrc]

We should also think about which options should be stable (vs unstable). I think we can start conservatively and only make max_width and tab_spaces stable. I think I would prefer to leave all the other formatting options unstable until 1.0 (the only one I can think of considering is hard_tabs).

That leaves the non-formatting options such as write_mode and skip_children. I think we can also be conservative here and leave them all till post-1.0.

[nrc]

So, a big question for me is what style of options we want to permit. Currently we have very fine-grained options, which makes Rustfmt very customisable, but:

• the chance of any set of options working together is low
• it is fiddly in the common case
• the set of options is huge and confusing

An alternative would be to support a small set of coarse grained styles. E.g., a compressed style would enable all the 'one line' options plus set wider values for the width heuristics, etc. We might support a style which always puts opening braces on a newline, but not any way to mix that with the 'braces on the same line' style.

We would aim to support each of these larger styles on their own, but combining them is at the user's own risk (and we would explicitly not test combinations).

If we don't do this, I think we probably need to combine options into groups in the documentation or something in order to make them less confusing.

Thoughts?

[nrc]

Some replies to @topecongiro's big list (thanks for doing that!):

• Brace: yes, agree
• Indent: yes to _indent for all of them. And I think 'yes' to just having block vs visual indent. However, I wonder if we should combine that with same line vs next line braces (e.g., not offer visual indent at all if you have next line braces)
• Match: I would remove indent_match_arm and I'm not sure about the last two either.
• Space: other than space_around_ranges I would combine these all into one option. (We might also come back and remove that one option later, we'll see) I'm not sure about type_punctuation_density, that could probably be removed.
• String: we should combine these two, I think
• Width: agree
• Sort items: I think we can have a single option - either we sort all the imports/extern crates in our chosen way or leave them as is.
• Legacy: let's just kill them, no need to deprecate
• Others: write_mode should probably not be user-visible. The motivation for report_* is that I wanted rustfmt to replace the make tidy checks in the Rust repo. I'm not sure whether or not we should try and do this kind of linting. In any case, they could be a single option.

[topecongiro]

control_style and where_style exists to support legacy style, so can we remove them as well?

[nrc]

control_style and where_style exists to support legacy style, so can we remove them as well?

Hmm, I'm not sure (and now I'm rethinking what to do about the legacy category). We might have a single overriding indent_style, which is either block or visual where block is the default and visual is a combination of the 'legacy' style options which works vaguely well.

OTOH, given the overwhelming support for the new style, I wonder if there is any need to provide a 'visual' style at all? Perhaps we do it as a first step and maybe remove later.

[topecongiro]

Hmm, I'm not sure (and now I'm rethinking what to do about the legacy category). We might have a single overriding indent_style, which is either block or visual where block is the default and visual is a combination of the 'legacy' style options which works vaguely well.
OTOH, given the overwhelming support for the new style, I wonder if there is any need to provide a 'visual' style at all? Perhaps we do it as a first step and maybe remove later.

Sounds good to me.

[nrc]

In a series of commits up to 86007e7, I have made a dent in reviewing the options. I think that I have removed all the low-hanging fruit and stabilised the options I want stable for 1.0. I've tried to organise the options a bit more in config.rs, once we're done we should extend that organisation to documentation.

Outstanding areas for concern:

• import ordering (see import reordering options #2185), once done, can probably also be a stable option
• semantics of comment_width (there is an issue for this somewhere)
• I would like to merge some of the single_line options, maybe
• we can probably trim some of the match options
• I still feel like there are too many spaces options, but I don't think we can combine further, perhaps we can kill one or two?
• I hope we can decide on nice formatting for combining control flow stuff and then we can remove combine_control_expr - I feel like it is only there for the sake of experimentation
• is fn_args_density useful?

[nrc]

With the latest bunch of commits, I've addressed everything I think needs to be addressed for 1.0, except #2185 (I'm leaving this issue on the milestone until that is fixed).

I think we should still address the questions in the above comment, but they only apply to unstable options and I believe we can make changes post-1.0.

[topecongiro]

@nrc Thank you very much!

[Luthaf]

Concerning the _indent family of settings, I used to have rustfmt set to use block for everything but chains, because I find long chains more readable with visual indent: I can jump from one call to the next without needing to scan to the left to find the call:

let result = some_variable.foo()
                          .do_with(|thing| thing.bar())
                          .filter(|a| a < bzz)
                          .unwrap_or(44)
                          .collect();

Vs:

let result = some_variable.foo()
    .do_with(|thing| thing.bar())
    .filter(|a| a < bzz)
    .unwrap_or(44)
    .collect();

I understand the need to remove options to keep the codebase sane, but is there any chance that this kind of setting might come back at some point, once 1.0 is released for example?

[nrc]

It is unlikely - visual indenting of chains has not been very popular. If there is a lot of demand, then it is possible, but that does seem unlikely to me.

[behnam]

The changes made here are great! ❤️

However, it was difficult to figure out how to update my personal and project .rustfmt to not get the warnings about unknown settings.

After realizing what happened, again it wasn't easy to update the config, because the old names are note listed in <Configurations.md>.

Would be great to either add the old names to <Configurations.md> and refer everyone there, or add a hint in the runtime warnings about the new name for each deprecated setting.

[nrc]

Would be great to either add the old names to <Configurations.md> and refer everyone there, or add a hint in the runtime warnings about the new name for each deprecated setting.

I think it would be good to do both!

[nrc]

Outstanding questions before I think we can take this off the milestone:

• do we need to stabilise use_small_heuristics - I feel some people dislike this and will want a way to turn it off. Since it is true by default, we are de facto stabilising the effect, if not the option.
• wrap_comments and normalize_comments are stable (but false by default). Formatting comments will change a lot and is pretty bad right now. Perhaps these should be unstable. If we keep them stable, should comment_width be stable too? (Seems weird for it to be used by wrap_comments but not be stable)
• Should the reorder_* items be stable? I think they should, my concern is whether we have exactly the right options and whether we'll end up adding many more in the future.
• Can we cut any of the spaces_* options? They are unstable so doesn't matter too much, but they are kind of annoying.
• remove_blank_lines_at_start_or_end_of_block could probably be stabilised
• I would like to stabilise match_arm_blocks but the name is bad
• should we stabilise use_field_init_shorthand?
• should unstable_features be stable? Obviously the concept is stable, but I'm not sure that allowing it in rustfmt.toml is the best idea
• I would like remove_nested_parens (Add remove nested parens option #2677) to be true by default, but that probably requires it being a stable option

[nrc]

On reflection, I think match_arm_blocks is not such a bad name, but we might want to combine it with force_multiline_blocks, so we won't stabilise for 1.0

[nrc]

Should the reorder_* items be stable? I think they should, my concern is whether we have exactly the right options and whether we'll end up adding many more in the future.

I'm stabilising reorder_modules and reorder_imports, but not reorder_impl_items because we might possibly combine it with a future reorder_items or something

[nrc]

remove_blank_lines_at_start_or_end_of_block could probably be stabilised

I actually think we could remove this option

[nrc]

Still outstanding:

• do we need to stabilise use_small_heuristics? - I feel some people dislike this and will want a way to turn it off. Since it is true by default, we are de facto stabilising the effect, if not the option. However there are multiple questions about the way we define the alternatives here.
• remove remove_blank_lines_at_start_or_end_of_block (always true)?

There are a bunch of unstable options I'd like to remove, but that can wait till post-1.0

[nrc]

I've made unstable_features unstable because there is an open question if we want to use the same configuration file for 'control' options as well as 'formatting' options.