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 information about supported formats to cli's output #771

Closed
ib opened this issue Jul 18, 2017 · 15 comments
Closed

Add information about supported formats to cli's output #771

ib opened this issue Jul 18, 2017 · 15 comments
Labels

Comments

@ib
Copy link

@ib ib commented Jul 18, 2017

This allows parsing the information if necessary and is informative to the user as well.

@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Jul 18, 2017

That's a good point @ib,
though, to be complete, we would probably have to mention
when cli supports .gz and .xz formats too

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Jul 18, 2017

though, to be complete, we would probably have to mention
when cli supports .gz and .xz formats too

This can already easily be parsed and seen from the cli's help output (--format=), but a new and compact Supports: line would be okay as well.

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Jul 18, 2017

Something like this:

*** zstd command line interface 32-bits v1.3.0, by Yann Collet
    supporting zstd formats v0.4+, gzip, lzma, xz ***

or this:

*** zstd command line interface 32-bits v1.3.0, by Yann Collet
*** supporting zstd formats v0.4+, gzip, lzma, xz
@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Jul 18, 2017

Yes, something like the 2nd example looks great

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Jul 18, 2017

Here you are.

@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Jul 18, 2017

Thanks @ib, it's pretty good,
though note that ZSTD_LEGACY_SUPPORT==0 means "no legacy support"

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Jul 19, 2017

Oops, missed the header (which btw had no range check so far).

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Jul 19, 2017

On top of that some beautification.

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Jul 21, 2017

@Cyan4973, are you going to apply this?

@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Jul 21, 2017

yes, it's in the list

@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Aug 18, 2017

The proposed patch makes zstdcli.c dependant on zstd_legacy.h.
However, it should be possible to compile zstdcli.c without legacy support, hence without availability of zstd_legacy.h.

Let's find another way...

@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Aug 19, 2017

In your patch, you propose this information to be displayed under usage_advanced(), which is triggered by -h command. In which case, it also displays additional instructions.

I was considering to trigger it under "verbose Version", which would be triggered by -vV. It would basically limit the output to the 2 lines mentioned.
What do you think ?

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Aug 19, 2017

-vV sounds good to me.

@ib

This comment has been minimized.

Copy link
Author

@ib ib commented Aug 19, 2017

This should do the job.

Cyan4973 added a commit that referenced this issue Aug 19, 2017
Requested and inspired by patch from @ib (#771)
@Cyan4973

This comment has been minimized.

Copy link
Contributor

@Cyan4973 Cyan4973 commented Aug 19, 2017

Thanks @ib .
#806 implements the requested feature, based on your suggested patch.
Branch displayVersion can be used to test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.