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,
+}