Skip to content

feat: implement display for Transaction#799

Merged
reez merged 1 commit intobitcoindevkit:masterfrom
ItoroD:implement-display
Jul 9, 2025
Merged

feat: implement display for Transaction#799
reez merged 1 commit intobitcoindevkit:masterfrom
ItoroD:implement-display

Conversation

@ItoroD
Copy link
Copy Markdown
Collaborator

@ItoroD ItoroD commented Jul 4, 2025

Description

I think we need to be able to see the details of a Transaction when printed. In Kotlin, Transaction currently prints the memory address and not the display. @reez and I had noticed this and discussed briefly, see here . I think we have manual implementations of display for struct like Address, so why not add it to Transaction too. Its a bit of a hassle, developing and testing and not being able to see the printout for Tx. I understand that swift is ok having just debug trait and can printout with no issues. This will at least for now allow JVM users see the actual printouts.

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@rustaceanrob
Copy link
Copy Markdown
Collaborator

I'm not sure it makes sense to derive the display trait for a debug string. The output won't be end-user friendly, so maybe this could be a "debug_string" method?

@ItoroD
Copy link
Copy Markdown
Collaborator Author

ItoroD commented Jul 7, 2025

I'm not sure it makes sense to derive the display trait for a debug string. The output won't be end-user friendly, so maybe this could be a "debug_string" method?

Yes, I figured. Thanks for confirming. But without display implementation we can't view or print Transaction in Kotlin and this way is probably not the best way to go about it.

Not sure how debug_string will help. Maybe in display implementation instead of forwarding the debug output, we can print a more user friendly transaction string. 😓 starting to sound "hacky"

reez

This comment was marked as outdated.

Copy link
Copy Markdown
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

ACK 2b5a771

@reez reez force-pushed the implement-display branch from 2b5a771 to 7d02c3e Compare July 9, 2025 16:40
@reez reez mentioned this pull request Jul 9, 2025
8 tasks
@ItoroD ItoroD force-pushed the implement-display branch from 7d02c3e to 0eac475 Compare July 9, 2025 17:02
@reez reez merged commit 0ffb921 into bitcoindevkit:master Jul 9, 2025
24 checks passed
@ItoroD ItoroD deleted the implement-display branch July 9, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants