-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-31648: Improve ElementPath #3835
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
Changes from all commits
d5ce7ac
3d4826b
8de5ec5
a3078f5
782ae97
29bbbcb
4879c72
26be846
6442ee3
86fb63c
140749f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2237,6 +2237,39 @@ def test_findall(self): | |
['tag'] * 2) | ||
self.assertEqual(e.findall('section//'), e.findall('section//*')) | ||
|
||
self.assertEqual(summarize_list(e.findall(".//section[tag='subtext']")), | ||
['section']) | ||
self.assertEqual(summarize_list(e.findall(".//section[tag ='subtext']")), | ||
['section']) | ||
self.assertEqual(summarize_list(e.findall(".//section[tag= 'subtext']")), | ||
['section']) | ||
self.assertEqual(summarize_list(e.findall(".//section[tag = 'subtext']")), | ||
['section']) | ||
self.assertEqual(summarize_list(e.findall(".//section[ tag = 'subtext' ]")), | ||
['section']) | ||
|
||
self.assertEqual(summarize_list(e.findall(".//tag[.='subtext']")), | ||
['tag']) | ||
self.assertEqual(summarize_list(e.findall(".//tag[. ='subtext']")), | ||
['tag']) | ||
self.assertEqual(summarize_list(e.findall('.//tag[.= "subtext"]')), | ||
['tag']) | ||
self.assertEqual(summarize_list(e.findall('.//tag[ . = "subtext" ]')), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't be enough to test just this variant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment applies. |
||
['tag']) | ||
self.assertEqual(summarize_list(e.findall(".//tag[. = 'subtext']")), | ||
['tag']) | ||
self.assertEqual(summarize_list(e.findall(".//tag[. = 'subtext ']")), | ||
[]) | ||
self.assertEqual(summarize_list(e.findall(".//tag[.= ' subtext']")), | ||
[]) | ||
|
||
# duplicate section => 2x tag matches | ||
e[1] = e[2] | ||
self.assertEqual(summarize_list(e.findall(".//section[tag = 'subtext']")), | ||
['section', 'section']) | ||
self.assertEqual(summarize_list(e.findall(".//tag[. = 'subtext']")), | ||
['tag', 'tag']) | ||
|
||
def test_test_find_with_ns(self): | ||
e = ET.XML(SAMPLE_XML_NS) | ||
self.assertEqual(summarize_list(e.findall('tag')), []) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,6 +157,9 @@ def prepare_predicate(next, token): | |
return | ||
if token[0] == "]": | ||
break | ||
if token == ('', ''): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it for whitespaces? Please add a comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
# ignore whitespace | ||
continue | ||
if token[0] and token[0][:1] in "'\"": | ||
token = "'", token[0][1:-1] | ||
signature.append(token[0] or "-") | ||
|
@@ -188,16 +191,22 @@ def select(context, result): | |
if elem.find(tag) is not None: | ||
yield elem | ||
return select | ||
if signature == "-='" and not re.match(r"\-?\d+$", predicate[0]): | ||
# [tag='value'] | ||
if signature == ".='" or (signature == "-='" and not re.match(r"\-?\d+$", predicate[0])): | ||
# [.='value'] or [tag='value'] | ||
tag = predicate[0] | ||
value = predicate[-1] | ||
def select(context, result): | ||
for elem in result: | ||
for e in elem.findall(tag): | ||
if "".join(e.itertext()) == value: | ||
if tag: | ||
def select(context, result): | ||
for elem in result: | ||
for e in elem.findall(tag): | ||
if "".join(e.itertext()) == value: | ||
yield elem | ||
break | ||
else: | ||
def select(context, result): | ||
for elem in result: | ||
if "".join(elem.itertext()) == value: | ||
yield elem | ||
break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a special test case for this bug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, it is already added! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already done. |
||
return select | ||
if signature == "-" or signature == "-()" or signature == "-()-": | ||
# [index] or [last()] or [last()-index] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
Improvements to path predicates in ElementTree: | ||
|
||
* Allow whitespace around predicate parts, i.e. "[a = 'text']" instead of requiring the less readable "[a='text']". | ||
* Add support for text comparison of the current node, like "[.='text']". | ||
|
||
Patch by Stefan Behnel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be enough to test just this variant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitebox testing, yes. Blackbox testing, no. It's short enough, so I chose the latter. In parsers, it's easy to accidentally forget about special cases.