Skip to content
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.

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

8.4: Document all new DOM classes and methods - part 1 #4212

Merged
merged 45 commits into from
Dec 7, 2024

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Dec 1, 2024

Very WIP, not ready for review yet.

TODO:

  • Update other extensions that link into DOM
  • Update constants
  • Finish class skeletons (class page, properties, ...)
  • Document the NO_DEFAULT_NS constant (with caution)
  • Document methods (NOTE: splitting PR because this is huge already)
  • Update note about encoding
  • Create examples of array indexing in NamedNodeMap ? Let's do this separately
  • Add a note on top of the dom book to guide users to the new classes? Do this later once the docs are completed
  • Explain xpath subtleties Separate PR
  • Add note to XMLDocument properties that you should rely on the LIBXML_ flags now

@nielsdos nielsdos force-pushed the dom-84 branch 2 times, most recently from b40772e to 6b116fb Compare December 1, 2024 15:49
@nielsdos nielsdos added this to the PHP 8.4 milestone Dec 1, 2024
@nielsdos nielsdos force-pushed the dom-84 branch 11 times, most recently from 5b1b19a to f762c1a Compare December 1, 2024 20:55
@nielsdos
Copy link
Member Author

nielsdos commented Dec 1, 2024

@Girgias I've finished with some basics. Most notably the class synopses are generated and the properties etc are all filled in. I've used xinclude quite a bit. I also fixed up some of the old DOM class descriptions.
It's probably best to already review this as-is, even though it's not finished and most notably the methods are missing.
This won't compile out of the box because of the entities.whatever.xml files not being generated because I haven't filled in the method stuff yet, but here's a zip that you can extract in reference/dom/dom to make it build:
entities.dom.zip
You also need php/doc-base#193

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Didn't test locally but this a pass across the whole PR

Comment on lines 278 to +315
<row xml:id="constant.domstring-size-err">
<entry>
<constant>DOMSTRING_SIZE_ERR</constant>
<constant>DOMSTRING_SIZE_ERR</constant> / <constant>Dom\STRING_SIZE_ERR</constant>
Copy link
Member

Choose a reason for hiding this comment

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

The issue I can see for this, is that automatic linking for <constant>Dom\STRING_SIZE_ERR</constant> is not going to work because the row has no corresponding XML ID.

Can an XML element have 2 IDs? If yes that would fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can an XML element have 2 IDs? If yes that would fix it.

No, but I don't think this is ever linked to directly 🤔

reference/dom/dom/dom-attr.xml Outdated Show resolved Hide resolved
reference/dom/dom/dom-attr.xml Show resolved Hide resolved
reference/dom/dom/dom-document.xml Show resolved Hide resolved
reference/dom/dom/dom-document.xml Outdated Show resolved Hide resolved
reference/dom/domcomment.xml Outdated Show resolved Hide resolved
reference/dom/domdocument.xml Outdated Show resolved Hide resolved
reference/dom/domdocumenttype.xml Outdated Show resolved Hide resolved
reference/dom/domentity.xml Outdated Show resolved Hide resolved
reference/xsl/xsltprocessor/transformtoxml.xml Outdated Show resolved Hide resolved
@nielsdos
Copy link
Member Author

nielsdos commented Dec 2, 2024

Thanks for the initial review. I pushed fixup commits so they can be validated individually, I'll later rebase this to merge the fixups with the original commits.

@nielsdos nielsdos changed the title [WIP] 8.4: Document all new DOM classes [WIP] 8.4: Document all new DOM classes and methods Dec 2, 2024
@nielsdos nielsdos force-pushed the dom-84 branch 3 times, most recently from 0463c12 to 1e6be82 Compare December 3, 2024 20:23
@nielsdos nielsdos merged commit 5ddcf09 into php:master Dec 7, 2024
2 checks passed
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