From cc22869fb45f5ad73dce67f8484dd2a6a61bd41e Mon Sep 17 00:00:00 2001 From: moxian Date: Fri, 29 Jun 2018 06:47:51 +0000 Subject: [PATCH 1/2] Add option to vertically align enum discriminants. --- Configurations.md | 46 +++++++++++++++++++ src/config/mod.rs | 2 + src/items.rs | 33 +++++++++++-- .../enum_discrim_align_threshold/20.rs | 28 +++++++++++ .../enum_discrim_align_threshold/20.rs | 28 +++++++++++ 5 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 tests/source/configs/enum_discrim_align_threshold/20.rs create mode 100644 tests/target/configs/enum_discrim_align_threshold/20.rs diff --git a/Configurations.md b/Configurations.md index 8861d7b2a136..6ac376efa682 100644 --- a/Configurations.md +++ b/Configurations.md @@ -885,6 +885,52 @@ impl Lorem { See also [`brace_style`](#brace_style), [`control_brace_style`](#control_brace_style). +## `enum_discrim_align_threshold` + +The maximum diff of width between enum variants to have discriminants aligned with each other. +Variants without discriminants would be ignored for the purpose of alignment. + +- **Default value** : 0 +- **Possible values**: any positive integer +- **Stable**: No + +#### `0` (default): + +```rust +enum Foo { + A = 0, + Bb = 1, + RandomLongVariantWithoutDiscriminant, + Ccc = 71, +} + +enum Bar { + A = 0, + Bb = 1, + ThisOneisWithDiscriminantAndPreventsAlignment = 10, + Ccc = 71, +} +``` + +#### `20`: + +```rust +enum Foo { + A = 0, + Bb = 1, + RandomLongVariantWithoutDiscriminant, + Ccc = 2, +} + +enum Bar { + A = 0, + Bb = 1, + ThisOneisWithDiscriminantAndPreventsAlignment = 10, + Ccc = 71, +} +``` + + ## `fn_single_line` Put single-expression functions on a single line diff --git a/src/config/mod.rs b/src/config/mod.rs index 52401a18d92e..a553bf2ee3c7 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -89,6 +89,8 @@ create_config! { combine_control_expr: bool, true, false, "Combine control expressions with function calls"; struct_field_align_threshold: usize, 0, false, "Align struct fields if their diffs fits within \ threshold"; + enum_discrim_align_threshold: usize, 0, false, + "Align enum variants discrims, if their diffs fit within threshold"; match_arm_blocks: bool, true, false, "Wrap the body of arms in blocks when it does not fit on \ the same line with the pattern of arms"; force_multiline_blocks: bool, false, false, diff --git a/src/items.rs b/src/items.rs index 46abe0867176..a341267681e6 100644 --- a/src/items.rs +++ b/src/items.rs @@ -500,6 +500,24 @@ impl<'a> FmtVisitor<'a> { let original_offset = self.block_indent; self.block_indent = self.block_indent.block_indent(self.config); + // If enum variants have discriminants, try to vertically align those, + // provided it does not result in too much padding + let pad_discrim_ident_to; + let diff_threshold = self.config.enum_discrim_align_threshold(); + let discr_ident_lens: Vec<_> = enum_def + .variants + .iter() + .filter(|var| var.node.disr_expr.is_some()) + .map(|var| rewrite_ident(&self.get_context(), var.node.ident).len()) + .collect(); + let shortest_w_discr = *discr_ident_lens.iter().min().unwrap_or(&0); + let longest_w_discr = *discr_ident_lens.iter().max().unwrap_or(&0); + if longest_w_discr > shortest_w_discr + diff_threshold { + pad_discrim_ident_to = 0; + } else { + pad_discrim_ident_to = longest_w_discr; + } + let itemize_list_with = |one_line_width: usize| { itemize_list( self.snippet_provider, @@ -514,7 +532,7 @@ impl<'a> FmtVisitor<'a> { } }, |f| f.span.hi(), - |f| self.format_variant(f, one_line_width), + |f| self.format_variant(f, one_line_width, pad_discrim_ident_to), body_lo, body_hi, false, @@ -543,7 +561,12 @@ impl<'a> FmtVisitor<'a> { } // Variant of an enum. - fn format_variant(&self, field: &ast::Variant, one_line_width: usize) -> Option { + fn format_variant( + &self, + field: &ast::Variant, + one_line_width: usize, + pad_discrim_ident_to: usize, + ) -> Option { if contains_skip(&field.node.attrs) { let lo = field.node.attrs[0].span.lo(); let span = mk_sp(lo, field.span.hi()); @@ -570,7 +593,11 @@ impl<'a> FmtVisitor<'a> { )?, ast::VariantData::Unit(..) => { if let Some(ref expr) = field.node.disr_expr { - let lhs = format!("{} =", rewrite_ident(&context, field.node.ident)); + let lhs = format!( + "{:1$} =", + rewrite_ident(&context, field.node.ident), + pad_discrim_ident_to + ); rewrite_assign_rhs(&context, lhs, &*expr.value, shape)? } else { rewrite_ident(&context, field.node.ident).to_owned() diff --git a/tests/source/configs/enum_discrim_align_threshold/20.rs b/tests/source/configs/enum_discrim_align_threshold/20.rs new file mode 100644 index 000000000000..d619d17922ae --- /dev/null +++ b/tests/source/configs/enum_discrim_align_threshold/20.rs @@ -0,0 +1,28 @@ +// rustfmt-enum_discrim_align_threshold: 20 + +enum Standard { + A = 1, + Bcdef = 2, +} + +enum Mixed { + ThisIsAFairlyLongEnumVariantWithoutDiscrim, + A = 1, + ThisIsAFairlyLongEnumVariantWithoutDiscrim2, + Bcdef = 2, +} + +enum TooLong { + ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaaaaaaaa = 10, + A = 1, + Bcdef = 2, +} + +// Live specimen from #1686 +enum LongWithSmallDiff { + SceneColorimetryEstimates = 0x73636F65, + SceneAppearanceEstimates = 0x73617065, + FocalPlaneColorimetryEstimates = 0x66706365, + ReflectionHardcopyOriginalColorimetry = 0x72686F63, + ReflectionPrintOutputColorimetry = 0x72706F63, +} \ No newline at end of file diff --git a/tests/target/configs/enum_discrim_align_threshold/20.rs b/tests/target/configs/enum_discrim_align_threshold/20.rs new file mode 100644 index 000000000000..06d621a34ccb --- /dev/null +++ b/tests/target/configs/enum_discrim_align_threshold/20.rs @@ -0,0 +1,28 @@ +// rustfmt-enum_discrim_align_threshold: 20 + +enum Standard { + A = 1, + Bcdef = 2, +} + +enum Mixed { + ThisIsAFairlyLongEnumVariantWithoutDiscrim, + A = 1, + ThisIsAFairlyLongEnumVariantWithoutDiscrim2, + Bcdef = 2, +} + +enum TooLong { + ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaaaaaaaa = 10, + A = 1, + Bcdef = 2, +} + +// Live specimen from #1686 +enum LongWithSmallDiff { + SceneColorimetryEstimates = 0x73636F65, + SceneAppearanceEstimates = 0x73617065, + FocalPlaneColorimetryEstimates = 0x66706365, + ReflectionHardcopyOriginalColorimetry = 0x72686F63, + ReflectionPrintOutputColorimetry = 0x72706F63, +} From 65ae0b9a094d83616585438b86e38b36047d8b45 Mon Sep 17 00:00:00 2001 From: moxian Date: Wed, 26 Sep 2018 18:29:26 +0000 Subject: [PATCH 2/2] Change enum align semantics to care about absolute length, rather than difference. If we're only aligning enum discriminants that are "not too far apart (length-wise)", then this works really well for enums with consistently-long or consistently-short idents, but not for the mixed ones. However, consistently-long idents is somewhate of an uncommon case and overlong idents may be allowed to be formatted suboptimally if that makes mixed-length idents work better (and it does in this case). --- Configurations.md | 25 +++++++------- src/items.rs | 21 ++++++------ .../{20.rs => 40.rs} | 16 ++++++--- .../enum_discrim_align_threshold/20.rs | 28 --------------- .../enum_discrim_align_threshold/40.rs | 34 +++++++++++++++++++ 5 files changed, 68 insertions(+), 56 deletions(-) rename tests/source/configs/enum_discrim_align_threshold/{20.rs => 40.rs} (52%) delete mode 100644 tests/target/configs/enum_discrim_align_threshold/20.rs create mode 100644 tests/target/configs/enum_discrim_align_threshold/40.rs diff --git a/Configurations.md b/Configurations.md index 6ac376efa682..824868bca098 100644 --- a/Configurations.md +++ b/Configurations.md @@ -887,9 +887,12 @@ See also [`brace_style`](#brace_style), [`control_brace_style`](#control_brace_s ## `enum_discrim_align_threshold` -The maximum diff of width between enum variants to have discriminants aligned with each other. +The maximum length of enum variant having discriminant, that gets vertically aligned with others. Variants without discriminants would be ignored for the purpose of alignment. +Note that this is not how much whitespace is inserted, but instead the longest variant name that +doesn't get ignored when aligning. + - **Default value** : 0 - **Possible values**: any positive integer - **Stable**: No @@ -897,18 +900,17 @@ Variants without discriminants would be ignored for the purpose of alignment. #### `0` (default): ```rust -enum Foo { +enum Bar { A = 0, Bb = 1, - RandomLongVariantWithoutDiscriminant, + RandomLongVariantGoesHere = 10, Ccc = 71, } enum Bar { - A = 0, - Bb = 1, - ThisOneisWithDiscriminantAndPreventsAlignment = 10, - Ccc = 71, + VeryLongVariantNameHereA = 0, + VeryLongVariantNameHereBb = 1, + VeryLongVariantNameHereCcc = 2, } ``` @@ -918,15 +920,14 @@ enum Bar { enum Foo { A = 0, Bb = 1, - RandomLongVariantWithoutDiscriminant, + RandomLongVariantGoesHere = 10, Ccc = 2, } enum Bar { - A = 0, - Bb = 1, - ThisOneisWithDiscriminantAndPreventsAlignment = 10, - Ccc = 71, + VeryLongVariantNameHereA = 0, + VeryLongVariantNameHereBb = 1, + VeryLongVariantNameHereCcc = 2, } ``` diff --git a/src/items.rs b/src/items.rs index a341267681e6..04e95935844a 100644 --- a/src/items.rs +++ b/src/items.rs @@ -501,22 +501,21 @@ impl<'a> FmtVisitor<'a> { self.block_indent = self.block_indent.block_indent(self.config); // If enum variants have discriminants, try to vertically align those, - // provided it does not result in too much padding - let pad_discrim_ident_to; - let diff_threshold = self.config.enum_discrim_align_threshold(); - let discr_ident_lens: Vec<_> = enum_def + // provided the discrims are not shifted too much to the right + let align_threshold: usize = self.config.enum_discrim_align_threshold(); + let discr_ident_lens: Vec = enum_def .variants .iter() .filter(|var| var.node.disr_expr.is_some()) .map(|var| rewrite_ident(&self.get_context(), var.node.ident).len()) .collect(); - let shortest_w_discr = *discr_ident_lens.iter().min().unwrap_or(&0); - let longest_w_discr = *discr_ident_lens.iter().max().unwrap_or(&0); - if longest_w_discr > shortest_w_discr + diff_threshold { - pad_discrim_ident_to = 0; - } else { - pad_discrim_ident_to = longest_w_discr; - } + // cut the list at the point of longest discrim shorter than the threshold + // All of the discrims under the threshold will get padded, and all above - left as is. + let pad_discrim_ident_to = *discr_ident_lens + .iter() + .filter(|&l| *l <= align_threshold) + .max() + .unwrap_or(&0); let itemize_list_with = |one_line_width: usize| { itemize_list( diff --git a/tests/source/configs/enum_discrim_align_threshold/20.rs b/tests/source/configs/enum_discrim_align_threshold/40.rs similarity index 52% rename from tests/source/configs/enum_discrim_align_threshold/20.rs rename to tests/source/configs/enum_discrim_align_threshold/40.rs index d619d17922ae..796e47c384ba 100644 --- a/tests/source/configs/enum_discrim_align_threshold/20.rs +++ b/tests/source/configs/enum_discrim_align_threshold/40.rs @@ -1,19 +1,25 @@ -// rustfmt-enum_discrim_align_threshold: 20 +// rustfmt-enum_discrim_align_threshold: 40 enum Standard { A = 1, Bcdef = 2, } -enum Mixed { - ThisIsAFairlyLongEnumVariantWithoutDiscrim, +enum NoDiscrims { + ThisIsAFairlyLongEnumVariantWithoutDiscrimLongerThan40, A = 1, - ThisIsAFairlyLongEnumVariantWithoutDiscrim2, + ThisIsAnotherFairlyLongEnumVariantWithoutDiscrimLongerThan40, Bcdef = 2, } enum TooLong { - ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaaaaaaaa = 10, + ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaaChar40 = 10, + A = 1, + Bcdef = 2, +} + +enum Borderline { + ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaa = 10, A = 1, Bcdef = 2, } diff --git a/tests/target/configs/enum_discrim_align_threshold/20.rs b/tests/target/configs/enum_discrim_align_threshold/20.rs deleted file mode 100644 index 06d621a34ccb..000000000000 --- a/tests/target/configs/enum_discrim_align_threshold/20.rs +++ /dev/null @@ -1,28 +0,0 @@ -// rustfmt-enum_discrim_align_threshold: 20 - -enum Standard { - A = 1, - Bcdef = 2, -} - -enum Mixed { - ThisIsAFairlyLongEnumVariantWithoutDiscrim, - A = 1, - ThisIsAFairlyLongEnumVariantWithoutDiscrim2, - Bcdef = 2, -} - -enum TooLong { - ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaaaaaaaa = 10, - A = 1, - Bcdef = 2, -} - -// Live specimen from #1686 -enum LongWithSmallDiff { - SceneColorimetryEstimates = 0x73636F65, - SceneAppearanceEstimates = 0x73617065, - FocalPlaneColorimetryEstimates = 0x66706365, - ReflectionHardcopyOriginalColorimetry = 0x72686F63, - ReflectionPrintOutputColorimetry = 0x72706F63, -} diff --git a/tests/target/configs/enum_discrim_align_threshold/40.rs b/tests/target/configs/enum_discrim_align_threshold/40.rs new file mode 100644 index 000000000000..3ed66039c9dd --- /dev/null +++ b/tests/target/configs/enum_discrim_align_threshold/40.rs @@ -0,0 +1,34 @@ +// rustfmt-enum_discrim_align_threshold: 40 + +enum Standard { + A = 1, + Bcdef = 2, +} + +enum NoDiscrims { + ThisIsAFairlyLongEnumVariantWithoutDiscrimLongerThan40, + A = 1, + ThisIsAnotherFairlyLongEnumVariantWithoutDiscrimLongerThan40, + Bcdef = 2, +} + +enum TooLong { + ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaaChar40 = 10, + A = 1, + Bcdef = 2, +} + +enum Borderline { + ThisOneHasDiscrimAaaaaaaaaaaaaaaaaaaaaa = 10, + A = 1, + Bcdef = 2, +} + +// Live specimen from #1686 +enum LongWithSmallDiff { + SceneColorimetryEstimates = 0x73636F65, + SceneAppearanceEstimates = 0x73617065, + FocalPlaneColorimetryEstimates = 0x66706365, + ReflectionHardcopyOriginalColorimetry = 0x72686F63, + ReflectionPrintOutputColorimetry = 0x72706F63, +}