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

Add a space between numbers and their units #345

Merged
merged 1 commit into from
Jan 2, 2022
Merged

Add a space between numbers and their units #345

merged 1 commit into from
Jan 2, 2022

Conversation

firasuke
Copy link
Contributor

@firasuke firasuke commented Jan 2, 2022

I think it's better to have some sort of separator (e.g. space) between numbers and their units.

@djc djc merged commit 8a126b2 into console-rs:main Jan 2, 2022
@firasuke firasuke deleted the patch-1 branch January 2, 2022 19:48
@firasuke
Copy link
Contributor Author

firasuke commented Jan 2, 2022

@djc thanks for merging this, I got a question though, shouldn't:

NumberPrefix::Standalone(number) => write!(f, "{:.0}B", number),

also be changed to:

NumberPrefix::Standalone(number) => write!(f, "{:.0} B", number),

I saw no change though when changing NumberPrefix::Standalone(number), perhaps because it's only changing the number and not the prefix? If so, why is there a B in there?

Thanks again!

@firasuke
Copy link
Contributor Author

firasuke commented Jan 2, 2022

Also it seems that some tests need to be modified after this changed.

@djc
Copy link
Member

djc commented Jan 2, 2022

Sorry, I clearly wasn't paying enough attention when I merged this. I reverted it in 20c5693, can you do another take to fix the CI issues?

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.

2 participants