Skip to content

Stop introspecting rawdata after Python 3.14.7 internal buffering added. #168

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.

Sign up for GitHub

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

Open
ianjosephwilson wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Stop introspecting rawdata after Python 3.14.7 internal buffering added. #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92c82dd
Stop introspecting rawdata after Python 3.14.7 internal buffering added.
ianjosephwilson Aug 27, 2026
ad214ea
Add test for issue 166.
ianjosephwilson Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 1 addition & 14 deletions tdom/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,24 +504,11 @@ def reset(self):
self.sinfo_table = {}

def close(self) -> None:
if self.waiting_for_data():
# We apply heuristics here to try to guess why the parser didn't finish.
if self.rawdata.count('"') % 2 == 1 or self.rawdata.count("'") % 2 == 1:

@davepeck davepeck Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind losing this.

That said, if we wanted to restore this behavior, I suppose we could:

  1. Join our entire (with-placeholders) string and call feed() exactly once, and remember its length
  2. Before calling close(), call getpos() and see if there's still a bit of content left
  3. Run something like this check on the remaining content

@ianjosephwilson ianjosephwilson Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result is pretty confusing, especially something like html(t'''<div id='1"></div>''') == ''.

It also breaks our placeholder tracking. Kind of a bummer. Maybe it won't happen very often because most people will be using an editor that will highlight the asymmetry of the quotes. I think the fact we are parsing small fragments is a disadvantage in this situation though because it exacerbates this type of mistake. This catches that we didn't use the placeholder but the user probably doesn't understand why: html(t'''<div id='{True}">''') raises ValueError: Some placeholders were never resolved..

A short test of your idea and it seems to work(!) but we are still implicitly depending on the implementation because we assume the buffering doesn't take place until after the first feed. We know that... because it says so in the source! Although if the threshold was higher then it might not parse at all. Seems like a really big implementation change on the stdlib's part which makes me nervous. Maybe this will just have to be a "gotcha" for a while...

I wonder where a good place to bring this up would be, a feature request on the issue tracker seems like it would get put on ice forever. Maybe starting a discussion on discuss.python.org? I'm an optimist on Thursdays.

raise ValueError(
"Parser expects more data, maybe you left an attribute quote unclosed?"
)
else:
raise ValueError(
"Parser expects more data, is the template valid html?"
)
super().close()

@ianjosephwilson ianjosephwilson Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An internal flush has to be forced here so super.close() is called at the top.

@davepeck davepeck Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if self.stack:
raise ValueError("Invalid HTML structure: unclosed tags remain.")
if self.source and self.source.has_placeholders():
raise ValueError("Some placeholders were never resolved.")
super().close()

def waiting_for_data(self):
return len(self.rawdata) > 0

# ------------------------------------------
# Getting the parsed node tree
Expand Down
16 changes: 0 additions & 16 deletions tdom/parser_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,22 +556,6 @@ def test_iter(self):
)


class TestIncompleteParsing:
def test_dangling_quotes(self):
with pytest.raises(ValueError, match="Parser expects more data"):
_ = parse_root(t"<div a='")
with pytest.raises(ValueError, match="Parser expects more data"):
_ = parse_root(t'<div a="')

def test_unfinished_attribute(self):
with pytest.raises(ValueError, match="Parser expects more data"):
_ = parse_root(t"<div a=")

def test_placeholder_missing_from_dangling_quote(self):
with pytest.raises(ValueError, match="Parser expects more data"):
_ = parse_root(t'<div a="{None}')


class TestComponentChildrenSpan:
@pytest.fixture
def Component(self):
Expand Down
6 changes: 6 additions & 0 deletions tdom/processor_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2273,3 +2273,9 @@ def test_mathml():
is not a decimal number.
</p>"""
)


def test_issue_166():
template = t"<button disabled={True}>x</button><button disabled={True}>y</button>"
expected = "<button disabled>x</button><button disabled>y</button>"
assert html(template) == expected