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

Hrp should have a constructor from a T: Display #195

Closed
apoelstra opened this issue Aug 5, 2024 · 3 comments · Fixed by #196
Closed

Hrp should have a constructor from a T: Display #195

apoelstra opened this issue Aug 5, 2024 · 3 comments · Fixed by #196

Comments

@apoelstra
Copy link
Member

This would allow easy construction from a rust-lightning RawHrp. Currently it allocates a String which is an unnecessary allocation and inconvenient.

@tcharding
Copy link
Member

How would such a constructor get at the string (bytes) created by the Display implementation without allocating a string?

We could add convenience constructors but I don't really see the value, what am I missunderstanding?

    /// Constructs a `Hrp` for an `hrp` string created by calling `hrp.to_string()`. 
    pub fn from_display(hrp: impl fmt::Display) -> Result<Self, Error> {
        let s = hrp.to_string();
        Self::parse(&s)
    }

    /// Constructs a `Hrp` for an `hrp` string created by calling `hrp.to_string()`. 
    pub fn from_display_unchecked(hrp: impl fmt::Display) -> Self {
        let s = hrp.to_string();
        Self::parse_unchecked(&s)
    }

@apoelstra
Copy link
Member Author

By implementing a fmt::Write backed by an array. I've almost finished this, I'll try to PR shortly.

apoelstra added a commit to apoelstra/rust-bech32 that referenced this issue Aug 6, 2024
apoelstra added a commit to apoelstra/rust-bech32 that referenced this issue Aug 6, 2024
@clarkmoody
Copy link
Member

Concept ACK, and cool implementation in #196

apoelstra added a commit that referenced this issue Aug 15, 2024
11d6cb4 hrp: add constructor from an arbitrary T: Display (Andrew Poelstra)

Pull request description:

  Fixes #195

ACKs for top commit:
  clarkmoody:
    ACK 11d6cb4

Tree-SHA512: 11c0ef91103c1df8a6a7aa46150a89d26e684b25ca9382fe0150eabdd58457709e585c55f5964c19698ff14e7c7e56f460206f362745d9226b7cc40a4412e4ad
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 a pull request may close this issue.

3 participants