Skip to content

Conversation

@HaoranYi
Copy link

@HaoranYi HaoranYi commented Mar 14, 2024

Problem

Per #241

pubkey display didn't honor user supplied format string. Inside fmt, it always use "{}". This commit fix it by using the fmt from the argument to format the string.

We probably should do this for all other user customized type formatter.

Summary of Changes

Fixes #

always use "{}". This commit fix it by using the fmt from the argument
to format the string.
@HaoranYi HaoranYi changed the title pubkey display didn't honor user supplied format string. Inside fmt it always use "{}". This commit fix it by using the fmt from the argument to format the string. Fix pubkey formatter Mar 14, 2024
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", bs58::encode(self.0).into_string())
let _ = bs58::encode(self.0).into_string().fmt(f);
write!(f, "")

Choose a reason for hiding this comment

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

is this some kind of "flush"?

Copy link
Author

Choose a reason for hiding this comment

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

Ah. Good catch. We don't need this. It was removed in the recent commit.

@joncinque
Copy link

This PR contains changes to the solana sdk, which will be moved to a new repo within the next week, when v2.2 is branched from master.

Please merge or close this PR as soon as possible, or re-create the sdk changes when the new repository is ready at https://github.com/anza-xyz/solana-sdk

Comment on lines +996 to +997
/// 1111111QLbz7JHiBTspS962RLKV8GndWFwiEaqKM (before)
/// 1111111QLbz7JHiBTspS962RLKV8GndWFwiEaqKMxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (this pr)
Copy link

Choose a reason for hiding this comment

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

comments don't really make sense post-merge. this kind of commentary belongs in pr description

@HaoranYi HaoranYi closed this Feb 4, 2025
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