Parser Error Messages

[ianjosephwilson]

Added parser error messages.

• Added exceptions
  • TemplatingError
  • ParsingError, ParsingAssertionError, AttributeParsingError
• TemplateParser improvements
  • Added tcomponent_children attribute
    • We have to parse these out to get "past" a component. We used to throw them away because we extract the children Template during processing. Now we keep them around during parsing for error handling to try to figure out why tags are not closed or don't match correctly, etc.
  • Added helper methods make_mismatch_error, make_invalid_endtag_error, run_unclosed_ambiguous_slash_checks, has_ambiguous_forward_slash and get_closed_tcomps
  • Used parsing exceptions in place of preexisting exceptions and cleaned up messages
  • Added more checks and more clear exceptions
• SourceReader changes
  • Added a few helper methods span_to_repr and span_to_template - these might get refactored later I just tried to matchup the older usage with the newer tools for now.

[ianjosephwilson]

The before parser (TemplateSpan) and after parser (TemplateRef) thing comes up when tags don't match because the starttag_span extracts from the original template whereas the endtag_ref are parsed by the template. I think this is fine for this case but I'm not sure if there actually is a way to reliably get the "end_pos" for some things. Here you can see the uncleaned up start tag is in the error but the cleaned up end tag is also there. When it actually parses they are both cleaned up and then processed out by tdom.

>>> html(t'<taga  x  =  1></tagb ..cruft >')
...
tdom.parser.ParsingError: Mismatched closing tag </tagb> at line 1 offset 15 for <taga  x  =  1> at line 1 offset 0.
>>>html(t'<taga  x  =  1></taga ..cruft >')
'<taga x="1"></taga>'
>>> 

[davepeck]

Thanks for banging through this! Left a couple initial comments but TBH I still don't have my head completely wrapped around it; will have more to say when I better understand how all the parts fit together.

[davepeck]

I'm wondering if we can simplify this.

Isn't the rule that if the raw text of the tag ends in />, but sinfo.startend is False, then it's an ambiguous forward slash? (We can safely remove the | None from sinfo:, I think?)

If the rule is that straightforward, then we don't need attrs at all...

[ianjosephwilson]

Yeah I saw this too I think maybe a few iterations ago there was no startend and now its in there and I can't tell if they are redundant or not (!!). I need to explore that and maybe clean it up.

The sinfo=None situation handles the situation where we are not tracking extra source information. I wanted to try to keep it optional in case it needed to be turned off for performance or whatever. This function still works and just says "no ambiguous slash here (because I couldn't check)". It isn't great but it keeps the "decision" all in one place.

[davepeck]

I think of TemplatingError as meaning "hey, that template you gave me is busted." That's a kind of exception our users will want to capture.

On the other hand, ParsingAssertionError sorta means "our parser is unexpectedly busted" which feels like it's not a TemplatingError.

I wonder if we should remove this and just use a grab-bag of approaches as appropriate:

• assert (for instance, assert starttag_text is not None in get_starttag_span(); shouldn't be possible inside TemplateParser unless there's a bug in our code)
• RuntimeError (for instance, if self.source is None in get_source(), which means TemplateParser was invoked from the outside in the wrong order)
• ??? if we hit an OpenTFragment in validate_end_tag(). Maybe a different choice of types would help clarify that this case can never happen?

[ianjosephwilson]

Yeah I think we need to play around with it when the processor handling is in too. We as library authors might want the same debugging information that the end-user would get with a caught message, ie. "our parser is unexpectedly busted ... and it got busted INSIDE the 15th nested component call during processing this 100 MB HTML file, etc.". I think some of these might be a bug in the HTMLParser itself or our expectations of it.

The Fragment issue I think is a refactor we need to make that I tried before and maybe after this settles out we'd go back in and try to sort that out. It comes up I think in other places as well.

[davepeck]

I'm still wrapping my head around this! Lots of machinery here, although it really is a gnarly case. Fun stuff and/or crazymaking stuff, can't decide which. 😅

[davepeck]

The before parser (TemplateSpan) and after parser (TemplateRef) thing comes up when tags don't match because the starttag_span extracts from the original template whereas the endtag_ref are parsed by the template. I think this is fine for this case

Yeah, I think it’s fine for this case too, but it’s a useful asymmetry to keep our eyes on. We have a reliable endtag_pos, but if we eventually want to, like, show carets beneath both tags like Rust error messages, we'll need to compute an endtag_span... somehow. Not sure how, though. For now, I think we live with this?