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 new feature to allow injection of an action into the extraction process to log and show progress. #225

Closed
wants to merge 3 commits into from

Conversation

gardebring
Copy link

Add new feature to allow injection of an action into the extraction process. This allows for showing or logging progress of the extraction process, especially useful for large files that might take a long time to extract.

…rocess. This allows for showing or logging progress of the extraction process, especially useful for large files that might take a long time to extract.
@adamhathcock
Copy link
Owner

There already events on Readers and Archives like FireFilePartExtractionBegin they may not cover what you're doing but there shouldn't be two mechanisms to listen to progress.

Can you use those events or do they not accomplish what you need? You can see the listener interface passed around where you've modified the code.

@gardebring
Copy link
Author

Hi Adam and thanks for your feedback. An event is, as you stated, indeed probably a better idea. It's a large codebase and I am not completely familiar with it yet, so I felt safer doing it the way I did, but I will have another look and see if I can do this as an event instead.

…action process. This allows for showing or logging progress of the extraction process, especially useful for large files that might take a long time to extract."

This reverts commit 467fc2d.
@gardebring
Copy link
Author

gardebring commented Apr 24, 2017

Alright, so a bit of an update here.
I can access the StreamListener from within AbstractReader, and it's easy to hookup a new event here.
However, once we move focus onto TransferTo in Utility we do loose that context and we cannot introduce the StreamListener into TransferTo since that method is used by other callers as well.

So my suggestion is:
I add a new EventHandler to IReader
I add a new Fire method to I IReaderExtractionListener to invoke this new event.
I pass the streamListener created in the WriteEntryTo method to the Write Method.
However, as a final step, I will not pass the streamListener to the TransferTo method (since this would introduce a dependency I think should be avoided), but instead I will here pass an optional Action, basically keeping the TransferTo method as my initial suggestion but moving all logic surrounding it so instead of passing down an action all the way we fetch it from the new event instead.

@adamhathcock
Copy link
Owner

I have questions but I guess I want to see what you try.

I'd like to have the IStreamListener interface be holistic in that it's the only thing used for events. How that looks can be up to you.

…l entry when extracting an archive. This allows for showing or logging progress of the extraction process, especially useful for large files that might take a long time to extract.
@gardebring
Copy link
Author

Hi again Adam. Please check out this commit and see if its more in line with your thoughts:
gardebring@683d271

This allows me to assign an event to the reader as such:
reader.EntryExtractionProgress += Reader_EntryExtractionProgress;

And then use it as such:
`
private int lastPercentage = -1;
//---//

private void Reader_EntryExtractionProgress(object sender, ReaderExtractionEventArgs e)
{
var progress = (SharpCompress.Readers.ReaderProgress)e.ParamList[0];
if (progress.PercentageRead % 10 == 0 && lastPercentage != progress.PercentageRead)
{
// Do something for every 10 percent of the extraction
}
}
`

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

2 participants