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 option to print size on the left #61

Merged
merged 5 commits into from
Mar 14, 2023

Conversation

bryceberger
Copy link
Contributor

@bryceberger bryceberger commented Mar 13, 2023

Added option to left-align disk usage

 --size-left                Show the size on the left, decimal aligned

@bryceberger
Copy link
Contributor Author

Ooh, just realized I forgot to add a test for combining left align and non-default precision. Should be implemented, just don't have a test for it. Will add tonight

Additionally, fix the implementation for that case
Copy link
Owner

@solidiquis solidiquis left a comment

Choose a reason for hiding this comment

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

Thanks for whipping this up. I won't be able to take a deeper look until later in the week as it's a work week, but I left some questions.

It also bothers me a bit that the root directory's disk usage isn't aligned with everyone else's. Would it be too difficult to offset that as well? Or perhaps there are other considerations that I'm missing.

src/render/tree/mod.rs Show resolved Hide resolved
src/render/tree/mod.rs Show resolved Hide resolved
@bryceberger
Copy link
Contributor Author

bryceberger commented Mar 13, 2023

It also bothers me a bit that the root directory's disk usage isn't aligned with everyone else's.

Not sure what you mean by this, everything is aligned in the output for me.

Maybe you were talking about the string literals in the size_left.rs test file? indoc didn't work because I wanted some indentation, and there isn't leading indentation on the first line because of the .trim() on line 25 of tests/utils/mod.rs

@bryceberger bryceberger marked this pull request as draft March 13, 2023 20:44
@solidiquis
Copy link
Owner

Ahh yes that was what I was referring to. In that case fantastic. I'll do a deeper review tonight! Also I did not mean to edit your comment above. I did not know that was even possible.

Copy link
Owner

@solidiquis solidiquis left a comment

Choose a reason for hiding this comment

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

Everything looks great! Could you just go through and add doc comments to all the places I noted? I know that it's pretty obvious what a lot of these things are doing but it's still something I wish to enforce to be thorough.

src/render/disk_usage.rs Show resolved Hide resolved
src/render/tree/node.rs Show resolved Hide resolved
src/render/tree/node.rs Show resolved Hide resolved
src/render/tree/node.rs Outdated Show resolved Hide resolved
src/render/tree/node.rs Outdated Show resolved Hide resolved
src/render/tree/node.rs Outdated Show resolved Hide resolved
@bryceberger bryceberger marked this pull request as ready for review March 14, 2023 03:41
@solidiquis solidiquis merged commit 7ee9946 into solidiquis:master Mar 14, 2023
@solidiquis
Copy link
Owner

Looks good to me! Thanks again for the contribution @bryceberger. For future reference if you wouldn't mind an update to the usage section of the README whenever new cli argument/options are introduced that'd be awesome. But I got it this time around :]

@solidiquis
Copy link
Owner

And wow this looks so nice :D

@solidiquis
Copy link
Owner

I included a screenshot in the README and decided to add --size-left as a default in my erdtree config. Thanks again!

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