-
Notifications
You must be signed in to change notification settings - Fork 850
feat: prefix a field with %% to use alternate Display
#3258
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
base: master
Are you sure you want to change the base?
feat: prefix a field with %% to use alternate Display
#3258
Conversation
d849f38 to
5c6b8f3
Compare
hds
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see anything in this change that looks like it's changing the behavior of the event and span macros to accept a %% prefix.
Is there something that wasn't added to the change?
(Caveat: I haven't actually tested the change, but without modifying the declarative macros I don't see how the new prefix could work)
Could you please clarify this?
Maybe I'm missing something but all changes in |
|
Yeah. I'm expecting changes in there. And some tests to go with it. Also, fair warning, we've recently found some inconsistencies in those macros and different field patterns. It needs to be cleaned up and covered in more comprehensive tests. To keep things sane, we probably won't merge this PR until that's done. |
Yeah, they're in the PR already though. |
|
@hds the changes you requested were already part of the PR |
|
@michaelbeaumont Sorry about that! I missed the changes as they were automatically folded up by GitHub. I'll have a look at this one, but after we make some general fixes to the macros where we've got a bunch of inconsistencies. |
Motivation
Fixes #1311
There's currently no way to use
{:#}to format a field, which is very important foranyhow::Errorin particular.Of course this should be backported v0.1
Solution
Add a new sigil
%%. I had parsing issues with#so I quickly abandoned that, though there may be a way to make it work. I'm not attached to this particular sigil at all.Add a new wrapper similar to
DisplayValuethat formats with the alternate#flag. The solution I have is useformat_args!, this should be OK AFAICT. I think ideally we would use theFormatterAPI to just switch on alternate, but it's nightly only.