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

human readable flag (-h) enabled #906

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sdorr0
Copy link

@sdorr0 sdorr0 commented Sep 17, 2023

This re-enables a quick, easy to type command line flag that ls users are familiar with for convenience.

In the event a user has changed the default configuration of lsd to use a size setting of something other than default, this lets a user use the historically established CLI flag (-h) to enable the human readable option without having to type --size=default on the command line.


TODO

  • Use cargo fmt
  • Add necessary tests
  • Add changelog entry
  • Update default config/theme in README (if applicable)
  • Update man page at lsd/doc/lsd.md (if applicable)

Scott Dorr added 4 commits September 16, 2023 17:38
This re-enables a quick, easy to type command line flag that `ls` users
are familiar with.

In the event a user has changed the default configuration of `lsd` to
use a size setting of something other than `default`, this lets a user
use the historically established CLI flag (`-h`) to enable the human
readable option without having to type `--size=default` on the command
line.
@muniu-bot
Copy link

muniu-bot bot commented Sep 17, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sdorr0

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zwpaper
Copy link
Member

zwpaper commented Sep 17, 2023

hi @sdorr0, thanks for the contribution.

I have one more question, what is users used -h with --size option? this would make --size not working?

@sdorr0
Copy link
Author

sdorr0 commented Sep 17, 2023

hi @sdorr0, thanks for the contribution.

I have one more question, what is users used -h with --size option? this would make --size not working?

Hmm. Yeah, the way the code is written now, -h will override a --size on the command line. I see that subsequent --size flags on the command line will override one another based on position (the 'last' one wins).

Never done Rust before. I'm not sure if there's a way to set up #[arg()] to also incorporate a short -h so it can play nicely with those positional dependencies, but I can research.

Would this be a requirement to merge this change in?

@sdorr0
Copy link
Author

sdorr0 commented Sep 17, 2023

I may have a solution

@sdorr0
Copy link
Author

sdorr0 commented Sep 17, 2023

Never mind. I've tried all kinds of approaches and can't seem to get clap to let -h work with --size=<mode> where the last one on the command line always wins.

So, yes, using -h will take precedence over a supplied --size=<mode> argument.

@zwpaper
Copy link
Member

zwpaper commented Sep 18, 2023

but it does not make sense for -h as a general flag to overwrite the --size as a specific flag.

many people may set the -h as default and it may lost --size if that

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