Skip to content

Add Stream Support for EPUB #64#70

Open
Flamifly wants to merge 3 commits intonissl-lab:masterfrom
Flamifly:Stream-Support-EPUB
Open

Add Stream Support for EPUB #64#70
Flamifly wants to merge 3 commits intonissl-lab:masterfrom
Flamifly:Stream-Support-EPUB

Conversation

@Flamifly
Copy link
Collaborator

Added EPUBHelper, which gets the EPUB Book for the ParserContext. In addition the Context will also validated.
EPUBMetaParser & EPUBTextParser uses the new Method to get the EPUB Book

Changed the RootNamespace to Toxy since it will always use ToxyFramework for every new File

@Flamifly
Copy link
Collaborator Author

Btw who is responsible to Dispose the Stream?

@tonyqus
Copy link
Member

tonyqus commented Mar 14, 2026

Developer should take care of the stream they passed in.

@tonyqus tonyqus changed the title adding Stream Support for EPUB #64 Add Stream Support for EPUB #64 Mar 14, 2026
@Flamifly
Copy link
Collaborator Author

I just saw i had a variable still in the code, which i used temporary to check the result, I removed it.

I also sealed the class and removed virtual form the context

@Flamifly Flamifly requested a review from tonyqus March 17, 2026 18:23
/// </summary>
/// <param name="context">Contains the Path to the EPUB File or the Stream.</param>
/// <returns>Returns the <see cref="EpubBook"/> in <paramref name="context"/>.</returns>
public static EpubBook GetEpubBook(ParserContext context)
Copy link
Member

@tonyqus tonyqus Mar 18, 2026

Choose a reason for hiding this comment

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

Perhaps you can put the method in Utility if the EPubHelper only has one method

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In my opinion there Utility is for more General things which are used more often (Validating Context, Get Stream, Checking if File is protected, ...)
To have a Method there which is used by 2 Parsers could be weird in my opinion.

If you want it there I can put it there

Is there anything else?
Otherwise I would merge

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.

2 participants