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

Format numbers #706

Closed
H4ckerxx44 opened this issue Jul 25, 2022 · 13 comments
Closed

Format numbers #706

H4ckerxx44 opened this issue Jul 25, 2022 · 13 comments
Assignees
Labels
enhancement New feature or request

Comments

@H4ckerxx44
Copy link

H4ckerxx44 commented Jul 25, 2022

Summary 💡

On larger code bases, the amount of files and the amount of lines of code could become a bit unreadable due to the lacking formatting, my feature request would be to format the lines of code, commits as well as the files datapoint like, 15614 -> 15,614 or 3255204307 -> 3,255,204,307

Motivation 🔦

That's 3 billlion lines of code, quite unreadable
grafik

@H4ckerxx44 H4ckerxx44 changed the title Formatting numbers Format numbers Jul 25, 2022
@spenserblack
Copy link
Collaborator

Just throwing out another option: what would you think about formats like 3K or 3.3M instead? Similar to the Size field.

@H4ckerxx44
Copy link
Author

Just throwing out another option: what would you think about formats like 3K or 3.3M instead? Similar to the Size field.

While I have nothing against that suggestion, I would not choose this due to it stripping away some "exactness" of the numbers at hand. (Let's ignore the Size in GiB)

@o2sh
Copy link
Owner

o2sh commented Jul 27, 2022

This feature would require to handle multiple formatting options (e.g. which thousands separator to use) and be able to detect the system local of the user (or use a cli flag to ask for the user's preference), num_format seems like a good candidate. Unfortunately the library doesn't seem to be maintained bcmyers/num-format#27

@spenserblack
Copy link
Collaborator

spenserblack commented Jul 27, 2022

TBH I'm not sure how necessary exactness is. This is just my opinion, but once billions of lines of code are reached, I wouldn't care if it's 3 billion, 3 billion and 1 hundred, or 3 billion and 1 thousand, I'd just care about the 3 or 4 most significant digits. If I'm showing off my repo, I'd be saying "look, over 3 billion lines of code!" not "look, 3 billion, 255 million, ..." 😆
Of course, for other outputs, like JSON, the exact number would be desired, but we wouldn't need to format it.

As @o2sh points out different locales have different thousands separators. And even the placement of the separators can differ (not all cultures historically group numbers into thousands), so any formatting with separators would need to come from a library.

@o2sh
Copy link
Owner

o2sh commented Oct 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@o2sh o2sh added the stale label Oct 30, 2022
@H4ckerxx44
Copy link
Author

Well, the activity depends on what you ultimately decide on for this idea.
I would just - as said previously - go with the 123,489,645 lines formatting of those numbers.

@o2sh o2sh removed the stale label Nov 6, 2022
@spenserblack
Copy link
Collaborator

Going through issues: since number format is a personal preference and/or localization issue, I think it should be a config option. So I'm going to suggest that #745 gets implemented before this. After that, I think the most reasonable thing is to offer some predefined named formatters that a user can pick in their config (defaulting to the "raw" format).

@o2sh
Copy link
Owner

o2sh commented Dec 9, 2022

I like this approach.
Does this mean we'll need to have a CLI flag --format-numbers in order to have it be part of the Config ? 🤔

@spenserblack
Copy link
Collaborator

spenserblack commented Dec 10, 2022

Depends on how the config is implemented. Preferably, we don't have to duplicate much work.

Honestly, we could make a "config" be purely a list of CLI args (I'm taking inspiration from .yardopts files). If we took this approach, we can continue our current style of making everything a CLI arg. Then we just parse the args that were in the file, then parse the args from the command-line, prioritizing CLI args over "optsfile" args where there's conflicts.

I suppose I'm kind of contradicting myself. I just said to wait until config files are implemented, but now I'm kind of arguing it should be a CLI arg 🤣

If this approach is worth it, I wonder if a crate already exists for this 🤔

@o2sh
Copy link
Owner

o2sh commented Dec 10, 2022

FWIW, it seems like num_format came back from the dead bcmyers/num-format#27 (comment) 😅

We should probably use it to format numbers. I'm in favor of adding a CLI flag to allow the user to specify the format with a default to No formatting.

The library comes a with a feature that can automatically detect the OS's locale information but it does not seem like a good idea to use it:

Since this type requires several dependencies (especially on Windows), it is behind a feature flag. To use it, include num-format = { version = "0.4.3", features = ["with-system-locale"] } in your Cargo.toml. Additionally, on Windows (but only on Windows), using SystemLocale requires Clang 3.9 or higher.

@spenserblack
Copy link
Collaborator

spenserblack commented Dec 10, 2022

FWIW, it seems like num_format came back from the dead

Good news! I'm all in favor of using it to format numbers 👍

@o2sh o2sh self-assigned this Dec 11, 2022
@o2sh
Copy link
Owner

o2sh commented Dec 14, 2022

Hi @H4ckerxx44, can you check if #892 resolves your issue?
Thanks.

@H4ckerxx44
Copy link
Author

Hi @H4ckerxx44, can you check if #892 resolves your issue? Thanks.

As I am not home right now I can't really check, though it seems like this does exactly what I had in mind.

Thanks for implementing it!

@o2sh o2sh added enhancement New feature or request and removed feature request labels Mar 11, 2023
@o2sh o2sh closed this as completed Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants