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

stat refactor #4150

Merged
merged 8 commits into from
Nov 19, 2022
Merged

stat refactor #4150

merged 8 commits into from
Nov 19, 2022

Conversation

tertsdiepraam
Copy link
Member

@tertsdiepraam tertsdiepraam commented Nov 17, 2022

This is an attempt to make the codebase of stat a bit nicer to work with. It consists of a couple of changes which I hope are relatively uncontroversial.

  1. Unit tests are moved to stat.rs so that we don't have to put pub in front of everything.
  2. I tried to reduce the complexity generally, by removing some weird intermediate variables, etc.
  3. I made the code more idiomatic Rust as opposed to C-like. For example, instead of modelling the formatting flags as a u8, I made a Flags struct with some bool fields. Also the case of no precision being provided is now modeled with None instead of -1.
  4. format! calls are preferred over some custom formatting function (e.g. extend_digits).
  5. OutputType now carries it's data in the original form (e.g. as u64) instead of as a preformatted string. This unifies all the formatting to print_it, instead of being scattered between print_it and do_stat.
  6. There's some minor cleanup across the file: fixing up comments, removing unnecessary type annotations, etc..

I think there's more to do, but I'm going to leave it here for this PR so it won't become too big.

Copy link
Collaborator

@jfinkels jfinkels left a comment

Choose a reason for hiding this comment

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

Nice

@sylvestre sylvestre merged commit 6d78505 into uutils:main Nov 19, 2022
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