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).

Author: moxian

Configurations.md

@@ -887,28 +887,30 @@ 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
 
 #### `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,
 }
 ```
 

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<usize> = 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(

tests/source/configs/enum_discrim_align_threshold/20.rs renamed to 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,
 }

tests/target/configs/enum_discrim_align_threshold/20.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,
+}