Parse nodes before and after the root element

[dubinsky]

@ashawley @SethTisue yet another bit of the lexical stuff

XML allows for comments and processing instructions to be present before the start and after the end of the root element. Currently, FactoryAdapter does not capture those nodes, and XMLLoader.loadXML does not provide access to anything other than the root element anyway. This pull request addresses the issue.

Note: at least with the JDK's Xerces, whitespace in the prolog and epilogue gets lost in parsing: the parser does not fire any white-space related events.

(Travis builds for JDK 17 fail with "no matching JDK found: 17" as noted in #559.)

[ashawley]

There's a lot of syntax fixes in this patch in files unrelated to the node parsing change. Is there a way to minimize this change so it can be reviewed easier?

[dubinsky]

Is there a way to minimize this change so it can be reviewed easier?

@ashawley done:

• semicolons dating from the time when this was (probably) re-written from Java restored ;)
• explicit types (in files unrelated to the node parsing change) removed;
• explicit override annotations removed, but not the note about a potentially problematic override situation that was discovered as the result of annotating the overrides.

Is this acceptable or should I minimize the patch some more?

Thanks!!!

[SethTisue]

(We're happy to have the code cleanups, if you still want to include them -- just in a separate commit from the main changes.)

[ashawley]

Yeah, I had no objection to the code cleanup, it is much needed. There was just a lot here. It's more ideal if bulk code cleanup were made separately in another PR.

[dubinsky]

Yeah, I had no objection to the code cleanup, it is much needed. There was just a lot here. It's more ideal if bulk code cleanup were made separately in another PR.

Cool. I agree that code cleanup should be done systematically, thoroughly and in bulk. But then there will be even more there ;)
Let's deal with the change at hand, then with #506 - and then maybe...

Thanks!

[dubinsky]

@ashawley Is there anything I need to change in this pull request for it to get merged? Thanks!

[SethTisue]

@dubinsky (this doesn't answer your question, but:) mind rebasing onto current main so we get GitHub Actions runs instead of Travis-CI?

[dubinsky]

mind rebasing onto current main so we get GitHub Actions runs instead of Travis-CI?

@SethTisue done

[dubinsky]

@ashawley @SethTisue Is there anything I need to change in this pull request for it to get merged? Thanks!

[SethTisue]

looks like scala/scala3#13744 should fix it

[dubinsky]

Yay! Thank you, I did not see that!

[dubinsky]

@ashawley @SethTisue Is there anything I need to change in this pull request for it to get merged? Thanks!

[SethTisue]

ping @ashawley — shall we just merge this?

[ashawley]

Yes, I think we should merge. I actually have a few amendments to the API choices in this around naming, but I can try to suggest those in a followup PR. I think the code for accepting and capturing any pre-processing elements or comments and such is something we should get working, regardless.

[dubinsky]

Thanks!