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

[fixed-hash] to_string() gives shortened version instead of full #656

Open
mo-rijndael opened this issue Jul 28, 2022 · 2 comments
Open

Comments

@mo-rijndael
Copy link

How to reproduce

here is snippet

// fixed-hash is 0.7.0 (latest)
use fixed_hash::construct_fixed_hash;
construct_fixed_hash! {struct H160(20);}

fn main() {
    let hash = H160::zero();
    // Debug formatting
    println!("{:?}", hash); // 0x0000000000000000000000000000000000000000

    // Display formatting
    println!("{}", hash); // 0x0000…0000

    // ToString
    println!("{}", hash.to_string()); // 0x0000…0000
}

Expected results

to_string() returns string representation of H160

Real results

to_string() returns shortened version

Source of problem

This occurs due to default implementation of ToString in standard library: impl<T: Display + ?Sized> ToString for T

Proposed solution

swap realizations for Debug and Display

Advantages

  • More suitable implementations of formatting traits (Shortened is useful for quick eye-checking, and when Display-ing to user we usually need full hash.)
  • Generates same amount of code, do not increase load to compiler

Disadvantages

  • It's a breaking change

Alternative solution

add codegen for ToString, which will return full version of hash

Advantages

  • Non-breaking change

Disadvantage

  • Generates more code, slowing compilation a bit
  • Leaves unsuitable implementation of formatting traits
@hexcowboy
Copy link

Also came here to argue in favor of this.

additional context: gakonst/ethers-rs#2175

@Kuly14
Copy link

Kuly14 commented Mar 16, 2023

add codegen for ToString, which will return full version of hash

Wouldn't the alternate solution also be a breaking change? If somebody already is using to_string() and counts that it will only give him the shortened version? Just curious.

If we could get this changed it would be great IMO.

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

No branches or pull requests

3 participants