Macro formatting

[andrewbaxter]

I think rust generally gives up with formatting macros, but it seems like some function-style macros will be automatically formatted.

log!("message", "a"="b", "c"="something else") will actually be wrapped and spaced prettily (not sure what my formatter is, I'm using the rust analyzer vs code plugin). => seems to block the formatter.

It's easy to work around (just copy the macros and replace => with =), and it would be a breaking change if introduced to the library, but I thought I'd just bring it up for thought.

Totally unrelated, but I came here after looking at tracing and I'd much prefer slog. Tracing has tons of undocumented magic, and (AFAICT) the use of globals/thread locals to do the magic then leads to issues like using spans with async code, etc. The fact that you pass loggers around explicitly is a huge selling point for me.

Anyways, I really appreciate the project!

[Techcable]

Yes. There's always a tradeoff between clarity and convenience 😉

I am not an expert on macro_rules! but the follow set ambiguity rules. This is why => is typically used for expressions. See indexmap::indexmap! macro for a popular example.

Since replacing => with another token could cause an ambiguity problem, it might be better to change the behavior of rustfmt. Especially since it's one of the only separators that can be used with expressions.

I have previously considered rewriting the slog macros as procedural macros to fix a couple (minor) bugs and improve clarity. I think this would allow us to work around the ambiguity, but I'm not sure.

[dpc]

We possibly could support both, and while technically possibly breaking, in practice it would just work, I think.

[andrewbaxter]

Yeah after diving in it was tricky coming up with a set of symbols that format properly but also allow expressing all the things slog does right now.

I've been using this modified set of macros in my projects -- I can't remember exactly what I did but I think I got rid of the string interpolation and tags, since the kvs are more important to me than interpolation

#[macro_export(local_inner_macros)]
macro_rules! o(
    ($($args:tt)*) => {
        slog::OwnedKV($crate::kv!($($args)*))
    };
);

#[macro_export(local_inner_macros)]
macro_rules! b(
    ($($args:tt)*) => {
        slog::BorrowedKV(&kv!($($args)*))
    };
);

#[macro_export(local_inner_macros)]
macro_rules! kv(
    (@ $args_ready:expr; $k:ident = %$v:expr) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin(@stringify $k), slog::__slog_builtin!(@format_args "{}", $v))), $args_ready); )
    };
    (@ $args_ready:expr; $k:ident = %$v:expr, $($args:tt)* ) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{}", $v))), $args_ready); $($args)* )
    };
    (@ $args_ready:expr; $k:ident = #%$v:expr) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{:#}", $v))), $args_ready); )
    };
    (@ $args_ready:expr; $k:ident = #%$v:expr, $($args:tt)* ) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{:#}", $v))), $args_ready); $($args)* )
    };
    (@ $args_ready:expr; $k:ident = ?$v:expr) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{:?}", $v))), $args_ready); )
    };
    (@ $args_ready:expr; $k:ident = ?$v:expr, $($args:tt)* ) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{:?}", $v))), $args_ready); $($args)* )
    };
    (@ $args_ready:expr; $k:ident = #?$v:expr) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{:#?}", $v))), $args_ready); )
    };
    (@ $args_ready:expr; $k:ident = #?$v:expr, $($args:tt)* ) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::__slog_builtin!(@format_args "{:#?}", $v))), $args_ready); $($args)* )
    };
    (@ $args_ready:expr; $k:ident = #$v:expr) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), slog::ErrorValue($v))), $args_ready); )
    };
    (@ $args_ready:expr; $k:ident = $v:expr) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), $v)), $args_ready); )
    };
    (@ $args_ready:expr; $k:ident = $v:expr, $($args:tt)* ) => {
        $crate::kv!(@ (slog::SingleKV::from((slog::__slog_builtin!(@stringify $k), $v)), $args_ready); $($args)* )
    };
    (@ $args_ready:expr; $kv:expr) => {
        $crate::kv!(@ ($kv, $args_ready); )
    };
    (@ $args_ready:expr; $kv:expr, $($args:tt)* ) => {
        $crate::kv!(@ ($kv, $args_ready); $($args)* )
    };
    (@ $args_ready:expr; ) => {
        $args_ready
    };
    (@ $args_ready:expr;, ) => {
        $args_ready
    };
    ($($args:tt)*) => {
        $crate::kv!(@ (); $($args)*)
    };
);

// Verbose, filled with details, not normally required
#[macro_export(local_inner_macros)]
macro_rules! trace(
    ($l:expr, $($args:tt)*) => {
        log!($l, slog::Level::Trace, "", $($args)*)
    };
);

// Decisions
#[macro_export(local_inner_macros)]
macro_rules! info(
    ($l:expr, $($args:tt)*) => {
        log!($l, slog::Level::Info, "", $($args)*)
    };
);

// Invalid behavior, but not critical
#[macro_export(local_inner_macros)]
macro_rules! warn(
    ($l:expr, $($args:tt)*) => {
        log!($l, slog::Level::Warning, "", $($args)*)
    };
);

// Critical failures
#[macro_export(local_inner_macros)]
macro_rules! err(
    ($l:expr, $($args:tt)*) => {
        log!($l, slog::Level::Error, "", $($args)*)
    };
);

#[macro_export(local_inner_macros)]
macro_rules! log(
    // `2` means that `;` was already found
   (2 @ { $($fmt:tt)* }, { $($kv:tt)* },  $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr) => {
      slog::Logger::log(&$l, &slog::record!($lvl, $tag, &slog::__slog_builtin!(@format_args $msg_fmt, $($fmt)*), $crate::b!($($kv)*)))
   };
   (2 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr,) => {
       log!(2 @ { $($fmt)* }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt)
   };
   (2 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr;) => {
       log!(2 @ { $($fmt)* }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt)
   };
   (2 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $($args:tt)*) => {
       log!(2 @ { $($fmt)* }, { $($kv)* $($args)*}, $l, $lvl, $tag, $msg_fmt)
   };
    // `1` means that we are still looking for `;`
    // -- handle named arguments to format string
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $k:ident = $v:expr) => {
       log!(2 @ { $($fmt)* $k = $v }, { $($kv)* slog::__slog_builtin!(@stringify $k) => $v, }, $l, $lvl, $tag, $msg_fmt)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $k:ident = $v:expr;) => {
       log!(2 @ { $($fmt)* $k = $v }, { $($kv)* slog::__slog_builtin!(@stringify $k) => $v, }, $l, $lvl, $tag, $msg_fmt)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $k:ident = $v:expr,) => {
       log!(2 @ { $($fmt)* $k = $v }, { $($kv)* slog::__slog_builtin!(@stringify $k) => $v, }, $l, $lvl, $tag, $msg_fmt)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $k:ident = $v:expr; $($args:tt)*) => {
       log!(2 @ { $($fmt)* $k = $v }, { $($kv)* slog::__slog_builtin!(@stringify $k) => $v, }, $l, $lvl, $tag, $msg_fmt, $($args)*)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $k:ident = $v:expr, $($args:tt)*) => {
       log!(1 @ { $($fmt)* $k = $v, }, { $($kv)* slog::__slog_builtin!(@stringify $k) => $v, }, $l, $lvl, $tag, $msg_fmt, $($args)*)
   };
    // -- look for `;` termination
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr,) => {
       log!(2 @ { $($fmt)* }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr) => {
       log!(2 @ { $($fmt)* }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, ; $($args:tt)*) => {
       log!(1 @ { $($fmt)* }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt; $($args)*)
   };
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr; $($args:tt)*) => {
       log!(2 @ { $($fmt)* }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt, $($args)*)
   };
    // -- must be normal argument to format string
   (1 @ { $($fmt:tt)* }, { $($kv:tt)* }, $l:expr, $lvl:expr, $tag:expr, $msg_fmt:expr, $f:tt $($args:tt)*) => {
       log!(1 @ { $($fmt)* $f }, { $($kv)* }, $l, $lvl, $tag, $msg_fmt, $($args)*)
   };
   ($l:expr, $lvl:expr, $tag:expr, $fmt:tt, $($args:tt)*) => {
       if $lvl.as_usize() <= slog::__slog_static_max_level().as_usize() {
           log!(1 @ { }, { }, $l, $lvl, $tag, $fmt; $($args)*)
       }
   };
   ($l:expr, $lvl:expr, $tag:expr, $fmt:tt) => {
       if $lvl.as_usize() <= slog::__slog_static_max_level().as_usize() {
           log!(1 @ { }, { }, $l, $lvl, $tag, $fmt; )
       }
   };
);

[Techcable]

We possibly could support both, and while technically possibly breaking, in practice it would just work, I think.

Yes. Dropping support for => is not an option because it would break backwards combat. Adding support for an alternate = syntax can't break working code.

Yeah after diving in it was tricky coming up with a set of symbols that format properly but also allow expressing all the things slog does right now.

I've been using this modified set of macros in my projects -- I can't remember exactly what I did but I think I got rid of the string interpolation and tags, since the kvs are more important to me than interpolation

[macro magic]

Thank you for this code! This should help working around the ambiguity with = 😉

I still think rewriting the macros as proc-macros is the right way to add this feature. The current macro implementation seems unclear and is prone to bugs (see #248).

Switching would also improve clarity and give better error messages.

Either way we end up implementing it, I appreciate the code.

[andrewbaxter]

FWIW I ended up writing a new rust formatter at https://github.com/andrewbaxter/genemichaels which formats macros decently and is a sufficient solution for me, so I'll close this.