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

Fix for issue 134 #135

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix for issue 134 #135

wants to merge 2 commits into from

Conversation

GlenKelley
Copy link

This fixed the issue filed in #134

This changes the behaviour from:

>formatPrefix("(~s", 1e3)(-1e3)`
(1)k

To be:

>formatPrefix("(~s", 1e3)(-1e3)`
(1k) 

A side effect of this change is we now include the unit to be included in the padding length, e.g:

>formatPrefix("(~s", 1e3)(-1e3)`
      −$42.0M

Becomes:

>formatPrefix("(~s", 1e3)(-1e3)`
     −$42.0M

@treloar
Copy link

treloar commented Jan 20, 2023

👍

@declanjscott
Copy link

declanjscott commented Jan 20, 2023

👍 thanks for this!

@GlenKelley
Copy link
Author

GlenKelley commented Jan 20, 2023

Note it would be cleaner if this was supported by the formatSpecifier, but that would require extending the format regex to support fixed suffix strings.

@GlenKelley
Copy link
Author

@mbostock What is the process for getting this change approved+merged?

@mbostock
Copy link
Member

mbostock commented Feb 1, 2023

I’ll get to it eventually. I appreciate your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants