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.

Sign up for GitHub

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

Jump to bottom

Fix Display width and precision of DisplayByteSlice #114

Merged
merged 3 commits into from
Oct 10, 2024
Merged

Fix Display width and precision of DisplayByteSlice #114

merged 3 commits into from
Oct 10, 2024

Conversation

tankyleo
Copy link
Contributor

@tankyleo tankyleo commented Oct 6, 2024 •
edited
Loading

Fixes #70

Also joins the implementation of Display for DisplayByteSlice and DisplayArray.

Partially addresses #83

tankyleo and others added 3 commits October 6, 2024 21:30
@tankyleo @tcharding
Fix width and precision of DisplayByteSlice
eb1599b 
Co-authored-by: Tobin C. Harding <[email protected]>
@tankyleo
Use same display impl for DisplayArray, DisplayByteSlice
169b9b0 
This is a code move.
@tankyleo
Apply same formatting tests to all Display impls
addb804 
At this point, these are `DisplayArray`, `DisplayByteSlice`.
@tankyleo
Copy link
Contributor Author

tankyleo commented Oct 6, 2024

No rush here.

  • Tobin, I added you as co-author to eb1599b, how does that sound ? You had worked on this previously, the difference here is that this honors odd-length precision.
  • The display of fmt_hex_exact is not yet consistent with DisplayByteSlice and DisplayArray. As a follow-up to this PR, we could have internal_display be written in terms of a T: IntoIterator, and have fmt_hex_exact_fn call internal_display

@tcharding
Copy link
Member

Tobin, I added you as co-author to eb1599b, how does that sound ? You had worked on this previously, the difference here is that this honors odd-length precision.

No attribution required, I'll do review for you though :)

apoelstra
apoelstra approved these changes Oct 7, 2024
View reviewed changes
Copy link
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 addb804 successfully ran local tests

tcharding
tcharding approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK addb804

@apoelstra apoelstra merged commit 60a88c1 into rust-bitcoin:master Oct 10, 2024
11 checks passed
@tankyleo tankyleo deleted the displayhex-impls branch October 14, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcharding tcharding tcharding approved these changes

@apoelstra apoelstra apoelstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

max is divided by 2
3 participants
@tankyleo @tcharding @apoelstra