Trailing commas

[ticki]

Some people prefer

enum {
    A,
    B,
    C,
}

quick_error should support that style in the macro.

[tailhook]

Well, initially I wanted commas to be required. But rust doesn't show good enough error message if you skip the comma. So we don't have commas.

Making comma optional probably means there will be $(,)* rule, which means many commas can be written. I'm not sure it's a good idea.

Another issue is that:

enum E {
  A,
  B,
}

.. looks good, but:

enum E {
  A {
    description("a")
  },
  B {
    description("b")
  },
}

... here trailing comma looks weird.

All in all, If we could make trailing commas more like in normal rust (i.e. only if there are no braces and optional trailing comma, but no more than one comma), it'd make more sense. With our limited set of instruments available in rust macros, the simpler rules are the better.

[ticki]

Oh, I tend to add trailing comma even when having variant fields ({}).

Somewhat more radical would be to make it a plugin instead of a normal macro_rules macro. That would allow arbitrary errors.

[tailhook]

Oh, I tend to add trailing comma even when having variant fields ({}).Oh, I tend to add trailing comma even when having variant fields ({}).

It's not about variant fields A {x: Type} it's about method shortcuts A { x: Type } { description("xxx") } which are usually multiline.

Somewhat more radical would be to make it a plugin instead of a normal macro_rules macro. That would allow arbitrary errors.

I always use stable rust. So it's not for me. I think macros 1.1 may be life-changer here, but I'm not sure.

Anyway, I'm not strictly against that. I'm just saying that I don't have time to investigate the issue deeply. Like find out all the cases where there is bad error reporting and figure out which way is better. Overall, I think it's minor issue that not worth doing all the work, until we have more powerful macros available. If you really really want to do all that work, we can figure out something.

[ticki]

Hmm, syntex might be of interest here.

[tailhook]

Code generation is still to much hassle for such a simple task.

[ticki]

I'm wondering if there is a general way to take a whole enum variant segment, such that special logic for handling trailing commas, comments, attributes, etc. isn't needed.

[tailhook]

No, there isn't one, as far as I know.

[kornelski]

I'd like to voice my support for optional commas, even at cost of accidentally allowing ,,,,.

• The error Rust prints for an extra comma is very confusing (something about [)
• I tend to use }, in structs, match, and a few other places, so I add these commas without thinking and keep running into this problem.

I've tried to add them myself, but the macro is …intimidating.

[jyn514]

Making comma optional probably means there will be $(,)* rule, which means many commas can be written. I'm not sure it's a good idea.

You could use $(,)?, which allows it exactly one time.

[tailhook]

Making comma optional probably means there will be $(,)* rule, which means many commas can be written. I'm not sure it's a good idea.

You could use $(,)?, which allows it exactly one time.

I remember that being quite fresh. I'm not sure if this is already old enough that we can use it without breaking anybody's code.

[jyn514]

This has been stable since 2018 edition, in 1.32. So I think it's safe to assume it will work on any modern toolchain.

[tailhook]

Yes, it's fine. Any volunteers for a PR?

[jyn514]

@tailhook it turned out not to be trivial:

Diff

diff --git a/src/lib.rs b/src/lib.rs
index e5eb62e..1839d19 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -195,6 +195,20 @@
 //!     }
 //! }
 //! ```
+//!
+//! As a convenience, `quick-error` allows trailing commas to enum variants.
+//!
+//! ```rust
+//! # use quick_error::quick_error;
+//! quick_error! {
+//!   enum E {
+//!     A,
+//!     B,
+//!     C,
+//!   }
+//! }
+//! ```
+//!
 //! # Context
 //!
 //! Since quick-error 1.1 we also have a `context` declaration, which is
@@ -298,14 +312,14 @@
 macro_rules! quick_error {
 
     (   $(#[$meta:meta])*
-        pub enum $name:ident { $($chunks:tt)* }
+        pub enum $name:ident { $($chunks:tt)* $(,)? }
     ) => {
         quick_error!(SORT [pub enum $name $(#[$meta])* ]
             items [] buf []
             queue [ $($chunks)* ]);
     };
     (   $(#[$meta:meta])*
-        enum $name:ident { $($chunks:tt)* }
+        enum $name:ident { $($chunks:tt)* $(,)? }
     ) => {
         quick_error!(SORT [enum $name $(#[$meta])* ]
             items [] buf []
@@ -313,7 +327,7 @@ macro_rules! quick_error {
     };
 
     (   $(#[$meta:meta])*
-        pub enum $name:ident wraps $enum_name:ident { $($chunks:tt)* }
+        pub enum $name:ident wraps $enum_name:ident { $($chunks:tt)* $(,)? }
     ) => {
         quick_error!(WRAPPER $enum_name [ pub struct ] $name $(#[$meta])*);
         quick_error!(SORT [enum $enum_name $(#[$meta])* ]
@@ -322,7 +336,7 @@ macro_rules! quick_error {
     };
 
     (   $(#[$meta:meta])*
-        pub enum $name:ident wraps pub $enum_name:ident { $($chunks:tt)* }
+        pub enum $name:ident wraps pub $enum_name:ident { $($chunks:tt)* $(,)? }
     ) => {
         quick_error!(WRAPPER $enum_name [ pub struct ] $name $(#[$meta])*);
         quick_error!(SORT [pub enum $enum_name $(#[$meta])* ]
@@ -330,7 +344,7 @@ macro_rules! quick_error {
             queue [ $($chunks)* ]);
     };
     (   $(#[$meta:meta])*
-        enum $name:ident wraps $enum_name:ident { $($chunks:tt)* }
+        enum $name:ident wraps $enum_name:ident { $($chunks:tt)* $(,)? }
     ) => {
         quick_error!(WRAPPER $enum_name [ struct ] $name $(#[$meta])*);
         quick_error!(SORT [enum $enum_name $(#[$meta])* ]
@@ -339,7 +353,7 @@ macro_rules! quick_error {
     };
 
     (   $(#[$meta:meta])*
-        enum $name:ident wraps pub $enum_name:ident { $($chunks:tt)* }
+        enum $name:ident wraps pub $enum_name:ident { $($chunks:tt)* $(,)? }
     ) => {
         quick_error!(WRAPPER $enum_name [ struct ] $name $(#[$meta])*);
         quick_error!(SORT [pub enum $enum_name $(#[$meta])* ]

---- src/lib.rs - (line 201) stdout ----
error: local ambiguity: multiple parsing options: built-in NTs tt ('chunks') or 1 other option.
 --> src/lib.rs:205:6
  |
7 |     A,
  |      ^