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

remove Argh/lean in favor of clap-only #215

Closed
Byron opened this issue Oct 11, 2021 · 3 comments · Fixed by #269
Closed

remove Argh/lean in favor of clap-only #215

Byron opened this issue Oct 11, 2021 · 3 comments · Fixed by #269
Assignees
Labels
good first issue Good for newcomers

Comments

@Byron
Copy link
Member

Byron commented Oct 11, 2021

cargo uses clap, so can we… it's not worth having multiple code paths for that and it's better to focus on one ultimate commandline arg parser rather than distribute efforts. It's possible to shrink clap for sure.

This is the foundation for fixing the long-standing issue of the progress being drawn after the program output was printed. The latter is already fixed for tui based progress, and the same solution should then work for the standard progress, too.

@Byron Byron assigned Byron and unassigned Byron Oct 11, 2021
@Byron Byron added the good first issue Good for newcomers label Oct 11, 2021
@oknozor
Copy link
Contributor

oknozor commented Dec 2, 2021

Hey I would be interested to pick this issue, to get familiar with the code base. Do you want clap only or clap with structopt ?

@Byron
Copy link
Member Author

Byron commented Dec 3, 2021

Thanks for your interest! Maybe I can give you a boost with this illustration:

Screen Shot 2021-12-03 at 8 05 26 AM

This issue isn't about implementing clap - it is available already - but removing the code path that uses argh. This work has already been performed for the porcelain CLI, but now needs to be done for the plumbing CLI as well.

I highly recommend following the collaboration guide, and if there are any additional questions, please let me know here or in the draft PR.

@oknozor
Copy link
Contributor

oknozor commented Dec 3, 2021

Thank you, I will give it a shot.

@Byron Byron self-assigned this Dec 5, 2021
Byron added a commit that referenced this issue Dec 5, 2021
Byron added a commit that referenced this issue Dec 5, 2021
- remove termion support for linux, use crossterm everywhere.
- remove additional 'light' intermediate targets
Byron added a commit that referenced this issue Dec 5, 2021
Byron added a commit that referenced this issue Dec 5, 2021
Byron added a commit that referenced this issue Dec 5, 2021
Byron added a commit that referenced this issue Dec 5, 2021
Otherwise the threaded line renderer will interfere with genuine
program output.
@Byron Byron closed this as completed in #269 Dec 5, 2021
Byron added a commit that referenced this issue Dec 6, 2021
…215)

Instead, read them eagerly and pass the result on to the closure
instead.
Byron added a commit that referenced this issue Dec 6, 2021
…he is used (#215)

Note that this is mostly for debugging or quickly seeing if object
caches help with certain operations.

Ideally the implementation knows themselves and sets up caches
accordingly, probably after trying it with these environment variables.
Byron added a commit that referenced this issue Dec 8, 2021
The previous name was too generic to be helpful or discoverable.
Byron added a commit that referenced this issue Dec 8, 2021
Byron added a commit that referenced this issue Dec 8, 2021
… by first commit only (#215)

Especially the sorting is useful to avoid having to sort commits by
hand after collecting them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants