Skip to content

Flush new pending data before checking if parser didn't finish. - #167

Closed
ianjosephwilson wants to merge 2 commits into
t-strings:mainfrom
ianjosephwilson:ian/draft_patch_for_issue166
Closed

Flush new pending data before checking if parser didn't finish.#167
ianjosephwilson wants to merge 2 commits into
t-strings:mainfrom
ianjosephwilson:ian/draft_patch_for_issue166

Conversation

@ianjosephwilson

Copy link
Copy Markdown
Contributor

I think this just gets things "working" for issue #166 but clearly it makes our dependency on the internals worse.

We use rawdata to check if the parser was able to complete parsing or if its waiting for more data. We also inspect it to see if we can guess why it couldn't finish. A common mistake is when a quote is left open like this: t'<div x="1></div>'.

The change in 3.14.7 adds a _pending buffer, list[str], that lets the parser delay actual parsing until a threshold is met whereas before the parser tried to parse everytime feed() was called. On close() this pending buffer is cleared out and parsing is done but the data is forced to '' which I don't quite understand but we cannot detect that it wasn't able to finish.

This patch flushes the pending buffer and runs a normal parse, self.goahead(0) (not self.gohead(1)), before checking self.rawdata during close().

@davepeck

Copy link
Copy Markdown
Contributor

Let's merge this now, but it feels like we should go back and do a deeper dive; sounds like we shouldn't be depending on rawdata to have this behavior.

@davepeck
davepeck marked this pull request as ready for review August 27, 2026 20:26
@ianjosephwilson

Copy link
Copy Markdown
Contributor Author

@davepeck Have a look at #168 it tries to just cut this out for now until we can find a way forward.

@ianjosephwilson
ianjosephwilson marked this pull request as draft August 27, 2026 20:39
@davepeck

Copy link
Copy Markdown
Contributor

@ianjosephwilson Agree; after looking at this a bit more, I think we need to avoid all use of rawdata, _pending, _pending_len, etc.

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