Skip to content

enable support for InclusiveNamespaces PrefixList in exclusive C14N spec... #55

Merged
merged 4 commits into from Aug 12, 2012

2 participants

@rogerhu
rogerhu commented Jun 22, 2012

Just wondering if you might have time to review/integrate. Given lxml's not able to support this right now we're forking our own custom version on our production sites because of it. Much appreciate the code review feedback! (we just fixed another bug that was found during our test runs in the last commit hash) Thanks!

@scoder
lxml member
scoder commented Aug 12, 2012

Looks mostly ok, thanks! I'll fix some minor memory handling issues and merge it.

@scoder scoder merged commit a48a3cd into lxml:master Aug 12, 2012
@scoder
lxml member
scoder commented Aug 12, 2012

It turns out that your changes didn't include any error tests. I guess that's why you ran into the problem in production. Since I've essentially fixed up your production code, you might want to make sure it keeps working by contributing another series of tests.

@rogerhu
@rogerhu
@scoder
lxml member
scoder commented Aug 14, 2012

I'm depending on a bug fix in Cython here that isn't in 0.16 yet. Basically, the user provided "const_xmlChar*" declaration isn't considered equivalent to "unsigned char*" by previous Cython versions and thus won't coerce automatically.

You can get a 0.17 pre-release from here:

http://wiki.cython.org/ReleaseNotes-0.17

I'll release another Cython beta (as in "release candidate") some time this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.