Skip to content

Conversation

@ThomasWaldmann
Copy link
Member

No description provided.

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Jan 24, 2026

Code fixes and commit comments created by AI, needs review.

@UlrichB22
Copy link

@ThomasWaldmann, can you please explain the problem behind this PR in a few words?
This would help the review.

@ThomasWaldmann
Copy link
Member Author

ThomasWaldmann commented Jan 26, 2026

@UlrichB22 added AI commit comments (Gemini 3 Pro (high)).

These were freshly generated, just looking at the changes.

Previously, when iter() encountered a non-Element child (like a text string), it would yield it unconditionally, even if the caller was searching for a specific tag (e.g., .//c).

```
    def test_Element_findall_dotslashslash():
        c1 = Element('c')
        c2 = Element('c')
        text = "text"
        b1 = Element('b', children=(c1, text, c2))
        b2 = Element('b')
        a1 = Element('a', children=(b1, b2, ))

        result = list(a1.findall('.//c'))
>       assert len(result) == 2
E       AssertionError: assert 3 == 2
E        +  where 3 = len([<Element 'c' at 1056c4450>, 'text', <Element 'c' at 1056c4810>])

src/emeraldtree/tests/test_tree.py:208: AssertionError
```

The fix ensures that non-Element children are only yielded if no tag filter is specified (tag is None).
This commit implements robust support for the .. (parent) XPath selector.

Tree Structure Changes: It adds a _parent attribute to the Element class.

The init method and all list-modification methods are updated to automatically maintain this parent pointer when children are modified.

Path Resolution: It updates ElementPath.py to use these _parent pointers when resolving.

If a parent pointer is missing (e.g., for objects not attached to the current context), it falls back to building a temporary parent map by traversing from the root.
@ThomasWaldmann
Copy link
Member Author

Found an issue, fixed it, force pushed.

@UlrichB22
Copy link

Thanks for the commit comments.

I understand the 'fix .// xpath queries' and successfully tested it.

The 'fix .// xpath queries' is too hard to understand by just looking at the code.
AFAIK Emeraldtree does not store parents and the function was not needed up to now. IMO we should just somehow disallow this query or issue a warning. We should not blindly trust the results of AI and the tests don't cover every situation.

If you like, I can test the first fix together with Moin and also include a break for the second query type.

- Implemented 1-based indexing for predicates (e.g. tag[1]).
- Raise SyntaxError for invalid indices (< 1) to match xml.etree behavior.
- Enabled test_Element_findall_position and added test_Element_findall_position_invalid.
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.

2 participants