Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add key-values to the macros #461

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Add key-values to the macros #461

merged 1 commit into from
Nov 5, 2021

Conversation

Thomasdezeeuw
Copy link
Collaborator

Attempt number two/three? Too many in any case.

Previously I proposed a design that followed a struct like syntax:

info!("my message: {}", arg, {
    key1: "value1",
    key2: 123,
});

However it turns out that this does not work well with named arguments
as reported in issues #369 and #372. The implementation was eventually
reverted in pr #374.

This new design takes inspiration from the tracing crate which already
supports key-value pairs in logging events. The basic idea is to put the
key-value pairs before the message and arguments. Applying the same
structure like syntax as above we would get something like the
following.

info!({
    key1: "value1",
    key2: 123,
}, "my message: {}", arg);

But personally I'm not a big fan of this formatting, let's try putting
everything on a single line instead.

info!({ key1: "value1", key2: 123 }, "my message: {}", arg);

A little better, but at this point the structure like syntax is really
more annoying then helpful. So, instead I've done away it, opting
instead use the following syntax.

info!(key1 = "value1", key2 = 123, "my message: {}", arg);

Two major differences:

  • Removed the brackets.
  • Colons (:) are replaced with equal/assignment signs (=).

This gives us syntax similar to variable assignment.

But then we run in some limitations of the macro syntax, specifically
that expr fragments aren't allowed after expr fragments. To fix this
I went with the easiest option of changing the last comma (,) after
the key-value pairs to a semicolon (;). Making the final syntax look
like the following.

info!(key1 = "value1", key2 = 123; "my message: {}", arg);
info!(target: "my_target", key1 = "value1", key2 = 123; "my message: {}", arg);
log!(target: "my_target", log::Level::Info, key1 = "value1", key2 = 123; "my message: {}", arg);

Which, in my opinion and all things considered, it's too bad looking.

/cc @KodrAus @yoshuawuyts

Attempt number two/three? Too many in any case.

Previously I proposed a design that followed a `struct` like syntax:

```rust
info!("my message: {}", arg, {
    key1: "value1",
    key2: 123,
});
```

However it turns out that this does not work well with named arguments
as reported in issues #369 and #372. The implementation was eventually
reverted in pr #374.

This new design takes inspiration from the `tracing` crate which already
supports key-value pairs in logging events. The basic idea is to put the
key-value pairs before the message and arguments. Applying the same
structure like syntax as above we would get something like the
following.

```rust
info!({
    key1: "value1",
    key2: 123,
}, "my message: {}", arg);
```

But personally I'm not a big fan of this formatting, let's try putting
everything on a single line instead.

```rust
info!({ key1: "value1", key2: 123 }, "my message: {}", arg);
```

A little better, but at this point the structure like syntax is really
more annoying then helpful. So, instead I've done away it, opting
instead use the following syntax.

```rust
info!(key1 = "value1", key2 = 123, "my message: {}", arg);
```

Two major differences:
 * Removed the brackets.
 * Colons (`:`) are replaced with equal/assignment signs (`=`).

This gives us syntax similar to variable assignment.

But then we run in some limitations of the macro syntax, specifically
that `expr` fragments aren't allowed after `expr` fragments. To fix this
I went with the easiest option of changing the last comma (`,`) after
the key-value pairs to a semicolon (`;`). Making the final syntax look
like the following.

```rust
info!(key1 = "value1", key2 = 123; "my message: {}", arg);
info!(target: "my_target", key1 = "value1", key2 = 123; "my message: {}", arg);
log!(target: "my_target", log::Level::Info, key1 = "value1", key2 = 123; "my message: {}", arg);
```

Which, in my opinion and all things considered, it's too bad looking.
@Thomasdezeeuw
Copy link
Collaborator Author

Ping @KodrAus.

@Thomasdezeeuw
Copy link
Collaborator Author

@sfackler you have the time to look at this?

@KodrAus
Copy link
Contributor

KodrAus commented Oct 26, 2021

Hey @Thomasdezeeuw! 👋 Sorry I hadn't gotten to this. My GitHub notifications have been a mess lately. I don't currently have commit access to log but can still review in the meantime.

We are a bit limited in what we can do with macro_rules, I think the place you're arriving at is not too bad indeed. The key = value syntax is more in line with format_args, but a little inconsistent with target. If anything though, I'd consider target the inconsistent one.

This looks good to me 👍

@KodrAus KodrAus merged commit 092aae0 into rust-lang:master Nov 5, 2021
@Thomasdezeeuw Thomasdezeeuw deleted the key-value-macros branch November 6, 2021 11:05
EFanZh pushed a commit to EFanZh/log that referenced this pull request Jul 23, 2023
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants