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

[MRG+1] xmliter nodename with dot #1533

Merged
merged 2 commits into from Oct 12, 2015
Merged

Conversation

@Digenis
Copy link
Member

@Digenis Digenis commented Oct 7, 2015

A dot is a valid character for an xml node name.
See http://www.w3.org/TR/2008/REC-xml-20081126/#NT-NameChar
Trying to match it leads to matching any character because it's not escaped.

nodename was treated like a regex which is a bug in itself
since it contradicts the function's doctring.

I'd rather discard the additional test.
Should I?

@kmike
Copy link
Member

@kmike kmike commented Oct 7, 2015

Out of curiosity, do you know if xmliter_lxml works corectly?

@Digenis
Copy link
Member Author

@Digenis Digenis commented Oct 7, 2015

No. I noticed some curiosities about namespaces but they have their tests.
Actually I found the above bug accidentally while looking for something else.
I was now going to ask whether I should add this test case to xmliter_lxml
but I see that being a subclass, it inherited already.

@kmike
Copy link
Member

@kmike kmike commented Oct 12, 2015

LGTM. This is technically backwards incompatible, so it deserves a note in release notes.

@kmike kmike changed the title Xml iter nodename with dot [MRG+1] xmliter nodename with dot Oct 12, 2015
curita added a commit that referenced this pull request Oct 12, 2015
[MRG+1] xmliter nodename with dot
@curita curita merged commit 0117c81 into scrapy:master Oct 12, 2015
0 of 2 checks passed
0 of 2 checks passed
codecov/patch CI failed: coverage not measured fully.
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@kmike kmike mentioned this pull request Oct 30, 2015
@redapple redapple added this to the v1.1 milestone Jan 25, 2016
@Digenis Digenis deleted the Digenis:xml_iter-nodename_with_dot branch Jan 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.