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

Allowing injecting custom-configured decoder instance #58

Closed
wants to merge 1 commit into from

Conversation

Seb-C
Copy link
Contributor

@Seb-C Seb-C commented Apr 10, 2021

Hello, and thank you for this package.

For my usecase, I need to set the decoder settings to Strict = false. I saw that you commented here, and made the suggested changes. If you were to merge this, you can probably close #27 .

Since the decoder needs to be injected a cachedReader, I did not find any other way than exposing the cachedReader object.

This change also happens to help with my usecase, because I want to be able to jump to any element from it's offset (during a stream-parse) with reader.Seek, which breaks the caching system unless I can empty it.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 92.64% when pulling e3dd943 on Seb-C:master into e73954f on antchfx:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 92.64% when pulling e3dd943 on Seb-C:master into e73954f on antchfx:master.

@zhengchun
Copy link
Contributor

exposing cachedReader is not good idea.

@Seb-C
Copy link
Contributor Author

Seb-C commented Apr 11, 2021

@zhengchun Can you please describe which design you would like for allowing customizing the decoder object (disabling strict mode for example)?
I implemented this design because you refused to merge the PR of @ninedraft (injecting options) and asked for a ParseWithDecoder design.
But this is not possible to implement this design without injecting a CachedReader into the decoder.

@zhengchun
Copy link
Contributor

cachedReader merged from PR #42, when #27 there was no cachedReader.

@zhengchun
Copy link
Contributor

Maybe now is the time to allow custom Decoder options like DecodeOptions(Strict,AutoClose,Entity) or add a global Decoder as default XML decoder to parsing, like http.DefaultClient

@Seb-C
Copy link
Contributor Author

Seb-C commented Apr 11, 2021

@zhengchun Thanks for your response. I understand.
I'll see if I can think about something else...

or add a global Decoder as default XML decoder to parsing, like http.DefaultClient

Actually I need to deal with multiple documents, and even multiple reader/decoder over the same file, so a singleton pattern is not suitable.

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.

3 participants