Skip to content

Migrate from oof to clap #108

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 20, 2021
Merged

Migrate from oof to clap #108

merged 5 commits into from
Oct 20, 2021

Conversation

SpyrosRoum
Copy link
Contributor

It's a single commit with many changes, sorry about that but I wasn't sure how to break it down.
Here are the help messages for reference:

$ ouch --help
ouch 0.2.0

Vinícius Rodrigues Miguel <vrmiguel99@gmail.com>:João M. Bezerra <marcospb19@hotmail.com>

A command-line utility for easily compressing and decompressing files and directories.

USAGE:
    ouch [OPTIONS] <SUBCOMMAND>

OPTIONS:
    -h, --help       Print help information
    -n, --no         Skip overwrite questions negatively
    -V, --version    Print version information
    -y, --yes        Skip overwrite questions positively

SUBCOMMANDS:
    compress      Compress files.    Alias: c
    decompress    Compress files.    Alias: d
    help          Print this message or the help of the given subcommand(s)

$ ouch c --help
ouch-compress 

Compress files.    Alias: c

USAGE:
    ouch compress <FILES>... <OUTPUT>

ARGS:
    <FILES>...    Files to be compressed
    <OUTPUT>      The resulting file. Its extensions specify how the files will be compressed
                  and they need to be supported

OPTIONS:
    -h, --help    Print help information

$ ouch d --help
ouch-decompress 

Compress files.    Alias: d

USAGE:
    ouch decompress [OPTIONS] <FILES>...

ARGS:
    <FILES>...    Files to be decompressed

OPTIONS:
    -h, --help               Print help information
    -o, --output <OUTPUT>    Decompress files in a directory other than the current

This closes #105

@SpyrosRoum
Copy link
Contributor Author

Oh I forgot to update the tests..

@figsoda
Copy link
Member

figsoda commented Oct 19, 2021

I can help adding completions and help2man man pages once this gets merged

@SpyrosRoum
Copy link
Contributor Author

Clap also has clap_generate which can be used but I'm not really sure what the best practices are

@figsoda
Copy link
Member

figsoda commented Oct 19, 2021

Yes that's what I meant to use, with a build.rs

@marcospb19
Copy link
Member

marcospb19 commented Oct 19, 2021 •
edited
Loading

Can't believe I just took a nap and this is already done.

It's a single commit with many changes, sorry about that but I wasn't sure how to break it down.

No problem! The small changes in src/cli.rs break everything anyway, so it's almost impossible to break it down during the process.

It's awesome, thanks for the effort, ouch has improved a ton since you guys started helping.

It's a +272 −855 change commit, and I did not found any errors, I'll pass the review to @vrmiguel, see if he has any comments about it.

@marcospb19 marcospb19 requested a review from vrmiguel October 19, 2021 16:25
@SpyrosRoum
Copy link
Contributor Author

Remembering all the different commands to compress and decompress all the different formats can be a huge pain and ouch is the first project I've seen that address this issue on the cli so I really want to help make it as good as possible

@marcospb19
Copy link
Member

I'm glad we feel the same about it :D.

@marcospb19
Copy link
Member

marcospb19 commented Oct 19, 2021 •
edited
Loading

Ok, I found a problem:

Vinícius Rodrigues Miguel <vrmiguel99@gmail.com>:João M. Bezerra <marcospb19@hotmail.com>

I hate seeing our names stand out in the --help message (it gives no "help") maybe we should instead have the link https://github.com/ouch-org/ouch at the end or beginning. It would be more useful.

Later we can consider adding it to the man pages, if necessary, @vrmiguel you agree on replacing it by the repository link?

figsoda
figsoda requested changes Oct 19, 2021
View reviewed changes
@figsoda
Copy link
Member

figsoda commented Oct 19, 2021

Also iirc required = true are not needed

@SpyrosRoum
Copy link
Contributor Author

Also iirc required = true are not needed

I believe you are correct but I wonted to be explicit since it's only a couple of times anyway

@marcospb19 marcospb19 mentioned this pull request Oct 19, 2021
Open
@figsoda
Copy link
Member

figsoda commented Oct 19, 2021 •
edited
Loading

this also creates another problem: clap uses atty and termcolor for NO_COLOR detection, which could cause inconsistencies with the current implementation

@marcospb19
Copy link
Member

marcospb19 commented Oct 19, 2021 •
edited
Loading

@figsoda you mean like, if one is detected and the other is not? (like ouch detects it and clap does not?).

@figsoda
Copy link
Member

figsoda commented Oct 19, 2021

yes

@marcospb19
Copy link
Member

marcospb19 commented Oct 19, 2021 •
edited by vrmiguel
Loading

Status update, pending:

  • Review from @vrmiguel.
  • Check if clap implementation for NO_COLOR detection is roughly compatible with ours.

@SpyrosRoum SpyrosRoum mentioned this pull request Oct 20, 2021
Merged
vrmiguel
vrmiguel approved these changes Oct 20, 2021
View reviewed changes
Copy link
Member

@vrmiguel vrmiguel left a comment

Choose a reason for hiding this comment

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

Seems great! Thanks!

I like our current help message better than the one clap gives us but I believe we can improve it later on using some template

@marcospb19 marcospb19 merged commit a46fa1f into ouch-org:master Oct 20, 2021
@marcospb19 marcospb19 added the hacktoberfest-accepted Tag PR as accepted for the hacktoberfest event label Oct 20, 2021
@SpyrosRoum SpyrosRoum deleted the migrate-to-clap branch October 21, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@figsoda figsoda figsoda requested changes

@vrmiguel vrmiguel vrmiguel approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted Tag PR as accepted for the hacktoberfest event
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate from oof to clap
4 participants
@SpyrosRoum @figsoda @marcospb19 @vrmiguel