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 progress to compressing/decompressing #210

Merged
merged 1 commit into from
Nov 25, 2021

Conversation

sigmaSd
Copy link
Contributor

@sigmaSd sigmaSd commented Nov 16, 2021

This pr uses the input size as an approximation for the compressed archive size.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 17, 2021

Some thoughts:

  • This appears to work well on my testing
  • I think I should also add a flag (no-progress-bar) to disable this, this is not only for users but for possible future interactive tests (should be straight forward, since I prepared for that with the display_handle api)
  • Decompressing could also have this, but Its trickier because the output can be multiple files (so not sure what to monitor here)

Obviously this is subjective, Its up to you if its a good addition or not

@sigmaSd sigmaSd changed the title Add progress to compressing Add progress to compressing/decompressing Nov 17, 2021
@marcospb19
Copy link
Member

I'm worried this conflicts a lot with #197

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 18, 2021

I can add a cli flag that restores the original behaviour, if this sound a good idea otherwise

@marcospb19
Copy link
Member

I can add a cli flag that restores the original behaviour, if this sound a good idea otherwise

Yeah, that would make merging with --accessibility much easier.

@SpyrosRoum
Copy link
Contributor

SpyrosRoum commented Nov 19, 2021

Oh this is great, I just decompressed a relatively large thing and I thought it was stuck for a moment so I was about to see if I can implement something like this myself haha

Note: I don't know if I misunderstood but adding a flag just for this and another --accessibility flag doesn't sound ergonomic, one --accessibility flag is better I think

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 19, 2021

That makes sense, I add --disable-progress-bar for now, so now the mechanism exists, the accessibility pr can just rename it, and repurpose it to its needs

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 19, 2021

Or you can merge the accessibility pr first and I'll rebase on top of it when I can

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 19, 2021

Btw does listing archive files needs a progress bar too?

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 19, 2021

Here is a re-based version https://github.com/sigmaSd/ouch/compare/progress...sigmaSd:acess?expand=1

Whats the point of a11y_show ? Shouldn't log! always check for ACCESSIBLE?

Edit: The rebase is on this branch to see it you need to compare progress vs acess branches

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 19, 2021

I went ahead and changed the flag to ACCESSIBLE in order to make the other pr conflicts as lest as possible

@marcospb19
Copy link
Member

Whats the point of a11y_show ? Shouldn't log! always check for ACCESSIBLE?

So, a11y_show is meant to disable some info!, because ouch outputs too much text, and this is not accessible.

And the ACCESSIBLE global boolean is to change how the information is displayed.

I also got confused about it, but the former one hides text, and the latter reformats it.

@marcospb19
Copy link
Member

The list command probably does not need a progress bar.

I think I'll wait for the other PR to finish before merging this one, if it takes too long, I guess this one can be merged before, not sure if it really makes a difference, tho.

@marcospb19
Copy link
Member

Accessibility PR was merged :D, now this one is no longer blocked, if you wish to resolve the conflicts.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Nov 25, 2021

Done

@sigmaSd sigmaSd changed the title Add progress to compressing/decompressing [WIP] Add progress to compressing/decompressing Nov 25, 2021
@sigmaSd sigmaSd changed the title [WIP] Add progress to compressing/decompressing Add progress to compressing/decompressing Nov 25, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants