fmt-rfcs icon indicating copy to clipboard operation
fmt-rfcs copied to clipboard

The requirement to merge the derive attribute was decided almost univocally

Open nagisa opened this issue 6 years ago • 0 comments

In this PR https://github.com/rust-dev-tools/fmt-rfcs/pull/105 a following requirement has been added to which I don’t agree:

There must only be a single derive attribute.

Is an unnecessary absolute which in certain corner cases will result in worse, more concise formatting than would otherwise be possible with multiple derive attributes were allowed. Consider:

-#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
-#[derive(serde_derive::Serialize, serde_derive::Deserialize)]
+#[derive(
+    Clone, Copy, Debug, PartialEq, PartialOrd, serde_derive::Serialize, serde_derive::Deserialize,
+)]

Where previous set up has the benefit of grouping along with being concise, the new setup spans more lines, does not immediately reveal any relationship between elements and it only gets worse as more derived traits are added:

-#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
-#[derive(serde_derive::Serialize, serde_derive::Deserialize)]
-#[derive(Banana, Peach)]
-#[derive(Who, What, When, Where, Why)]
+#[derive(
+    Clone,
+    Copy,
+    Debug,
+    PartialEq,
+    PartialOrd,
+    serde_derive::Serialize,
+    serde_derive::Deserialize,
+    Banana,
+    Peach,
+    Who,
+    What,
+    When,
+    Where,
+    Why,
+)]

Furthermore, when comments get involved, things get even sketchier:

 // In current implementation this gets merged!
-#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
-#[derive(serde_derive::Serialize, serde_derive::Deserialize)]
-#[derive(Banana, Peach)]
-#[derive(Who, What, When, Where, Why)] // Derive the 5Ws
+#[derive(
+    Clone,
+    Copy,
+    Debug,
+    PartialEq,
+    PartialOrd,
+    serde_derive::Serialize,
+    serde_derive::Deserialize,
+    Banana,
+    Peach,
+    Who,
+    What,
+    When,
+    Where,
+    Why,
+)] // Derive the 5Ws

 // But this does not, violating the rule for any reasonable interpretation of MUST or ONE
-#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
-#[derive(serde_derive::Serialize, serde_derive::Deserialize)] // Derive serde
-#[derive(Banana, Peach)] // Derive fruits
-#[derive(Who, What, When, Where, Why)]
+#[derive(
+    Clone, Copy, Debug, PartialEq, PartialOrd, serde_derive::Serialize, serde_derive::Deserialize,
+)] // Derive serde
+#[derive(Banana, Peach)] // Derive fruits
+#[derive(Who, What, When, Where, Why)]

There doesn’t appear to be any discussion pertaining this issue (or any discussion at all, really) on this kind of rule that I could find, only this merged PR. Should we have a discussion or remove the requirement from the normative specification?

nagisa avatar Jun 17 '19 21:06 nagisa