Skip to content

Add impl_fmt_traits macro#90

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:05-16-fmt
May 20, 2024
Merged

Add impl_fmt_traits macro#90
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:05-16-fmt

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented May 16, 2024

Add a macro that implements the fmt::{LowerHex, UpperHex, Display, Debug} traits. Includes support for displaying backwards.

Context

In bitcoin_hashes we have two versions of this code, moving it here is better because it is general hex related code. Note however that the reason for supporting display backwards might see strange unusual to the casual reader.

Note

Note please the use of naked core::foo in the macro requiring core to be in scope, is this reasonable of should we do the ugly re-export stuff we do in bitcoin_hashes: $crate::_export::_core::foo?

@tcharding tcharding changed the title Add hex_fmt_impl macro Add impl_fmt_traits macro May 16, 2024
@tcharding
Copy link
Copy Markdown
Member Author

PR is tested in rust-bitcoin/rust-bitcoin#2770

@tcharding tcharding force-pushed the 05-16-fmt branch 2 times, most recently from 52d907b to a06b7ed Compare May 16, 2024 04:10
@tcharding tcharding changed the title Add impl_fmt_traits macro Add impl_fmt_traits! macro May 16, 2024
@tcharding tcharding changed the title Add impl_fmt_traits! macro Add impl_fmt_traits macro May 16, 2024
@apoelstra
Copy link
Copy Markdown
Member

In a06b7ed:

Concept ACK. But

  • We probably should do the ugly core stuff, since it guards against the case where a user has their own core module in scope.
  • We should change the interface to look more Rust-like, e.g.
impl_fmt_traits! {
    #[reverse(true)] // or whatever
    impl<T: Whatever> all_fmt_traits for MyType<T> {
        const LENGTH: usize = 32;
    }
}

@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented May 16, 2024

woops, macro layout stuff sill to come. All review suggestions implented.

@tcharding tcharding force-pushed the 05-16-fmt branch 2 times, most recently from 4ba1693 to 7ddf1c0 Compare May 17, 2024 00:08
Add a macro that implements the `fmt::{LowerHex, UpperHex, Display,
Debug}` traits. Includes support for displaying backwards.
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cb9b86a I am a tiny bit tempted to say that the const LENGTH should wind up as an actual associated constant. Though this is easy to add in a followup PR

@apoelstra apoelstra merged commit 7102dba into rust-bitcoin:master May 20, 2024
@tcharding tcharding deleted the 05-16-fmt branch May 20, 2024 22:43
@Kixunil
Copy link
Copy Markdown
Collaborator

Kixunil commented Jun 30, 2024

@apoelstra that's impossible because you can't write [u8; Self::LENGTH * 2] and declaring the constant anyway is weird since none of the traits have it.

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.

3 participants