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

Load memoryStream using CopyAsync #58

Open
wants to merge 1 commit into
base: dev-pr
Choose a base branch
from

Conversation

JohnLeyva
Copy link

Make it work with Blazor WAsm

@rabanti-github rabanti-github added the code review A PR or patch is recently reviewed label Feb 25, 2024
@rabanti-github rabanti-github changed the base branch from master to dev-pr February 25, 2024 02:54
@rabanti-github rabanti-github changed the base branch from dev-pr to dev February 25, 2024 02:54
@rabanti-github rabanti-github changed the base branch from dev to dev-pr February 25, 2024 02:55
Copy link
Owner

@rabanti-github rabanti-github left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.
A small change about the exception handling is necessary to ensure that only one type of exception is exposed to the higher level of the library.

Also: May this modification also be necessary für the writer function? I assume the main change is this here:

                await Task.Run(() =>
                {
                    ReadZip(zf);
                }).ConfigureAwait(false);

@@ -70,32 +70,81 @@ public XlsxReader(Stream stream, ImportOptions options = null)
/// </exception>
public void Read()
{
try
Copy link
Owner

Choose a reason for hiding this comment

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

The try block as wrapper is still necessary since FileStream could fail with several other Exceptions than a common IOException (An own version is used anyway --> Exceptions.IOException). The only Exception that is used as interface is this mentioned IOException.
See also: .NET documentation about possible exception for FileStream

@rabanti-github rabanti-github removed the code review A PR or patch is recently reviewed label Mar 8, 2024
@rabanti-github rabanti-github added the change request A PR needs some attention label Apr 13, 2024
@rabanti-github
Copy link
Owner

The feature to load workbooks as stream or file asynchronously was implemented in Release v2.4.0.
I had to adapt some things, but I hope, I have implemented your PR feature correctly.
Because of the adaptions, this PR will not be merged. However, your are mentioned in the changelog as origin of the adaptions.
Please let me know in a new issue, when something is not working as expected.
Note: This PR will be closed within the next weeks or months.
Thanks, again for the contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change request A PR needs some attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants