SalOne22 / rimage

This is CLI tool inspired by squoosh!
Apache License 2.0
306 stars 19 forks source link

[Feature]: Improve args input #220

Open Mikachu2333 opened 5 months ago

Mikachu2333 commented 5 months ago

Description

This command could run successfully ./_rimage.exe moz 1717128900569_u.jpg -q 90 -s _update

But this can't, because error: unexpected argument '-q' found tip: to pass '-q' as a value, use '-- -q' ./_rimage.exe png 1717128900569_u.jpg -q 90 -s _update

Problem or Motivation

Some of the command would like to use -- -q rather than -q for example, and should be improved.

Expected behavior

Run successfully

Alternatives Considered

It seemed that only moz mode would run without errors.

Anything else?

No response

SalOne22 commented 5 months ago

Check warning on method that used when parsing multiple positional args - https://docs.rs/clap/latest/clap/builder/struct.Arg.html#method.num_args This is a internal behavior of clap cli parsing. Possible ways to fix it:

SalOne22 commented 5 months ago

This is a internal behavior of clap cli parsing. Possible ways to fix it:

And this issue is not related to clap cli parsing, but more to clap's way of showing tips. png command doesn't accept --quality as option, so clap panics with this error: error: unexpected argument '-q' found. But for some reason it shows tip that doesn't make sense tip: to pass '-q' as a value, use '-- -q'. One way to fix this is to remove suggestions completely, but I will try find better solution.

UPD: -- -q passes -q as input file

SalOne22 commented 5 months ago

in dd2fe3e added warning when file is not found, should I also set default log level to warn? This will add additional information to the user about what happened. But warn level also will show warnings like WARN zune_jpeg::decoder > Marker 0xFFED not known

Mikachu2333 commented 5 months ago

should I also set default log level to warn?

I think there's no need to do so for normal users.

SalOne22 commented 5 months ago

Then we should add to #200 note that if any error is occurred, user need to consult the help menu. Some lossy codecs can accept quality, lossless not.

For reference, lossy codecs - mozjpeg, regular jpeg, avif and webp. Other doesn't have --quality option available.

Mikachu2333 commented 5 months ago

Then we should add to #200 note that if any error is occurred, user need to consult the help menu. Some lossy codecs can accept quality, lossless not.

Agree.


Besides, I wonder where would --preserve arg be added into, common.rs also? If not, please note me about the file, and I'll try to improve help info in #200.

And, maybe the help info should note some Key Content that users should pay attention to and should be emphasized.

e.g. jxl Before: Only support lossless output, ... (detailed infomation) After: Only support lossless output, ... (detailed infomation)

Is there a way to implement infos with this display style?

Mikachu2333 commented 5 months ago

Some lossy codecs can accept quality, lossless not.

In my option, just panic!() or give the error info and exit is enough. If users add the -q option for lossless codec.