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

Options that require --long should either imply it or be silently ignored #152

Closed
elyscape opened this issue Mar 31, 2017 · 7 comments
Closed

Comments

@elyscape
Copy link

Currently, if you try to execute exa with a long view option but without specifying --long/-l, it gives an error:

scapeless:exa elyscape$ exa -@
Option --extended is useless without option --long.
scapeless:exa elyscape$ exa -l@
.rw-r--r--   22k elyscape 30 Mar 21:18 Cargo.lock
.rw-r--r--   724 elyscape 30 Mar 21:18 Cargo.toml
.rw-r--r--  1.1k elyscape 30 Mar 21:18 LICENCE
.rw-r--r--   759 elyscape 30 Mar 21:18 Makefile
.rw-r--r--@ 5.5k elyscape 30 Mar 21:18 README.md
                                       └── someattr (len 9)
.rw-r--r--  7.3k elyscape 30 Mar 21:18 Vagrantfile
drwxr-xr-x     - elyscape 30 Mar 21:18 contrib
.rw-r--r--  454k elyscape 30 Mar 21:18 screenshots.png
drwxr-xr-x     - elyscape 30 Mar 21:18 src
drwxr-xr-x     - elyscape 30 Mar 21:18 xtests

For comparison, here's what happens on macOS with ls:

scapeless:exa elyscape$ ls -@
Cargo.lock	LICENCE		README.md	contrib		src
Cargo.toml	Makefile	Vagrantfile	screenshots.png	xtests
scapeless:exa elyscape$ ls -l@
total 1000
-rw-r--r--   1 elyscape  staff   22074 Mar 30 21:18 Cargo.lock
-rw-r--r--   1 elyscape  staff     724 Mar 30 21:18 Cargo.toml
-rw-r--r--   1 elyscape  staff    1080 Mar 30 21:18 LICENCE
-rw-r--r--   1 elyscape  staff     759 Mar 30 21:18 Makefile
-rw-r--r--@  1 elyscape  staff    5511 Mar 30 21:18 README.md
	someattr	     9 
-rw-r--r--   1 elyscape  staff    7275 Mar 30 21:18 Vagrantfile
drwxr-xr-x   4 elyscape  staff     136 Mar 30 21:18 contrib
-rw-r--r--   1 elyscape  staff  454908 Mar 30 21:18 screenshots.png
drwxr-xr-x   9 elyscape  staff     306 Mar 30 21:18 src
drwxr-xr-x  29 elyscape  staff     986 Mar 30 21:18 xtests

That is to say, if -@ is specified without -l, it is silently ignored. Rather than error out, this still gives some useful input. That being said, it makes a certain amount of sense to assume that, in the event a user has specified, say, --git, the user probably wants long output and that it should be automatically turned on.

@lilyball
Copy link
Contributor

I agree that it should go ahead and output anyway. I don't think it should imply --long though. Perhaps the best approach is to print the current warning about the option being useless, followed up by the normal output (as if the useless option wasn't specified).

@Mange
Copy link

Mange commented Jul 10, 2017

Perhaps the best approach is to print the current warning about the option being useless

Then one cannot have an alias with the common options they want and only specify -l when they want long output.
Common example: alias ls='ls -h' for those that want human sized by default when giving -l.

Things like --git could also support the short format in the future (like a colored marker after filenames), which means it should probably not imply --long automatically, or else a --short option has to be added later.

@lorenzos
Copy link

lorenzos commented Aug 4, 2017

+1 mainly because what @Mange said about aliases. I'm using aliases to "replace" ls with exa --git every time.

@ogham
Copy link
Owner

ogham commented Aug 4, 2017

It's not ready for release yet, but in the option-pars-ng branch (the hardest part of development is coming up with the branch name), there's been a lot of work to support this case.

Basically, exa kept on running into limits of the default getopts crate: it didn't accept invalid UTF-8, it threw an error on repeated options, and doesn't support flags later in the list overriding ones from earlier, like how ls does it. I looked at the options, decided that the world needed one more, and wrote a custom option parser that works the "right" way. With this in place, exa will start to ignore options that require --long unless that option is passed in to.

I wrote some more stuff on it in one of the files if you're interested in the details.

@lilyball
Copy link
Contributor

lilyball commented Aug 4, 2017

Nice. I would suggest the usage of https://clap.rs instead of doing something custom, except apparently clap-rs doesn't have any way to allow later arguments to override earlier ones. There's already an issue open about this (clap-rs/clap#976), so I'm going to go comment on that now.

@lorenzos
Copy link

lorenzos commented Aug 11, 2017

I updated the repo, but looks the fatal warning is still there:

# git log --oneline 
97d1472 Merge branch 'strict-mode-properly'
b286676 Add actual error messages for the error messages
adaa36e Integrate strict mode, use it to test file sizes
dbebd60 Extract var_os and use the mock to test
532ebbc Only complain about long options in strict mode

# make clean
cargo clean

# make
cargo build --release --no-default-features --features "default"
   Compiling [...]
   Compiling exa v0.7.0 (file:///home/lorenzo/Dev/exa)
    Finished release [optimized] target(s) in 93.23 secs

# target/release/exa --git
Option --git is useless without option --long (-l).

@ogham
Copy link
Owner

ogham commented Aug 11, 2017

Argh I forgot to change it for the --git flag

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

No branches or pull requests

5 participants