Adding support for external Callback

lestrrat-go / libxml2

Interface to libxml2, with DOM interface

MIT License

230 stars 55 forks source link

Adding support for external Callback #77

Closed AMCR closed 11 months ago

AMCR commented 3 years ago

This PR contains the following features:

adding support for XML Callback - this new feature will allow us to use the new Go1.16 embed feature when handling XSDs;

Additionally, the code includes the following refactoring:

using Go error wrapping instead of github.com/pkg/errors
improving stringToXMLChar to also return the function to free the allocated memory;
adding function stringToChar to convert Go String to C char* and the function to free the allocated memory;
updating some tests;

lestrrat commented 3 years ago

As it is, it's a bit hard to review your changes. Can you cleanup your tree, and also can you please separate the main change from the following?

using Go error wrapping instead of github.com/pkg/errors

improving stringToXMLChar to also return the function to free the allocated memory;

adding function stringToChar to convert Go String to C char* and the function to free the allocated memory;

updating some tests;

lestrrat commented 3 years ago

(Also, obviously the tests need to be passing)

lestrrat commented 3 years ago

@AMCR Please check my previous comment

AMCR commented 3 years ago

@lestrrat Thanks for your comments. I will address then ASAP

AMCR commented 3 years ago

Hey @lestrrat! I have cleaned up the tree as you asked. I will propose the other changes in a different PR.

lestrrat commented 3 years ago

I will propose the other changes in a different PR.

Thanks! Give me a little time to tweak your change. I got the intention behind the change, but there are few things that I want to do on my side that has to do with style and other intended future changes.

AMCR commented 3 years ago

Thanks for your suggestions. They make a lot of sense. Let me address the issues mentioned above.

AMCR commented 3 years ago

I've decided to use the following implementation instead:

type Callback interface {
   Open(string) (io.ReadClose, error)
   CallbackMatcher
}

This way, if called concurrently, the Open operation will return a different io.Reader.

lestrrat commented 3 years ago

@AMCR oh, was I to review the code already? Sorry, I knew you updated this PR, but I didn't think that it would be ready so quickly. Sorry, but I'm out of tuits for the day. I will check back sometime tomorrow my time.

github-actions[bot] commented 11 months ago

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] commented 11 months ago

This PR was closed because it has been stalled for 14 days with no activity. This does not mean your PR is rejected, but rather it is done to hide it from the view of the maintainers for the time being. Feel free to reopen if you have new comments or chnages that you would like to include.