Skip to content

dom: Add stubs generated from php-src stubs#2813

Merged
Girgias merged 45 commits into
php:masterfrom
Girgias:stub-8.3-dom
Nov 12, 2023
Merged

dom: Add stubs generated from php-src stubs#2813
Girgias merged 45 commits into
php:masterfrom
Girgias:stub-8.3-dom

Conversation

@Girgias
Copy link
Copy Markdown
Member

@Girgias Girgias commented Sep 30, 2023

Generate using php/php-src#12330 and running the following command (for my folder hierarchy):

../php-src/build/gen_stub.php --generate-methodsynopses ../PHP-8.3/ext/dom/ ../docs-php/en/reference/dom/

<term><parameter>nodes</parameter></term>
<listitem>
<para>
Description.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to leave these empty instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, ideally, people would work on them directly after generating the stubs. And if no text is generated, one gets an empty tag, which I don't think is great.

@Girgias Girgias force-pushed the stub-8.3-dom branch 5 times, most recently from 314d435 to d3bba5e Compare September 30, 2023 19:08
@ndossche
Copy link
Copy Markdown
Member

ndossche commented Oct 1, 2023

A question.
The after() method on DOMElement etc come from the interface DOMChildNode. Looking at that page I see after() listed there: https://www.php.net/manual/en/class.domchildnode.php
Clicking on after() reveals its documentation, albeit a bit lackluster. It's also hard to discover these methods right now.

The after() method (and others coming from the interface) is not listed on https://www.php.net/manual/en/class.domelement.php
So how do we go about it? I guess the description should be the same for all of them. It's a bit weird that the interface has a page tbh?

@Girgias
Copy link
Copy Markdown
Member Author

Girgias commented Oct 1, 2023

Oh so that's where they are documented.

I mean one solution is to add:

    <xi:include xpointer="xmlns(db=http://docbook.org/ns/docbook) xpointer(id('class.domparentnode')/db:refentry/db:refsect1[@role='description']/descendant::db:methodsynopsis[@role='DOMParentNode'])">
     <xi:fallback/>
    </xi:include>

In the inherited method section, maybe having a look at how countable classes do it? As Countable::count also has a page

@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented Oct 1, 2023

It's also hard to discover these methods right now.

Indeed. I've ran into this exact issue myself before: https://chat.stackoverflow.com/transcript/message/56491752#56491752

Comment thread reference/dom/domelement/before.xml
Comment thread reference/dom/domdocumentfragment/append.xml
@ndossche
Copy link
Copy Markdown
Member

ndossche commented Nov 9, 2023

These still remain and I'll try to finish them soon:

  • replaceWith
  • replaceChildren
  • getIterator

Thanks for having a look already

@ndossche
Copy link
Copy Markdown
Member

ndossche commented Nov 9, 2023

Looks like I'm getting some errors because of the xincludes:

File /home/runner/work/doc-en/doc-en/en/reference/dom/domcharacterdata/after.xml No parameters sections.
File /home/runner/work/doc-en/doc-en/en/reference/dom/domcharacterdata/after.xml No returnvalues sections.

are those checked before resolving the xincludes by any chance?

@Girgias
Copy link
Copy Markdown
Member Author

Girgias commented Nov 10, 2023

Looks like I'm getting some errors because of the xincludes:

File /home/runner/work/doc-en/doc-en/en/reference/dom/domcharacterdata/after.xml No parameters sections.
File /home/runner/work/doc-en/doc-en/en/reference/dom/domcharacterdata/after.xml No returnvalues sections.

are those checked before resolving the xincludes by any chance?

Yes, they don't actually work on the manual but individual files. Just go and add exceptions to the sections_check QA script. As it probably should be written to work on the full manual.

@ndossche ndossche marked this pull request as ready for review November 11, 2023 16:00
@ndossche
Copy link
Copy Markdown
Member

I think this is ready for review now.
I submitted php/doc-base#106 to fix the integration.

Copy link
Copy Markdown
Member Author

@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.

LGTM except the formatting of the error sections which I am not sure.

variablelist feels inappropriate, I usually think if doing it that way we use itemizedlist but DocBook seems to be OK with using it that way.

Now the wrapping para tag is unnecessary and should be removed.

The last thin is that it would be nice to have the descriptions for the errors somehow only written once and included so that they don't go out of sync.

Comment thread reference/dom/domchildnode/after.xml Outdated
Comment on lines +51 to +53
Raised if the parent is of a type that does not allow children of the
type of one of the passed <parameter>nodes</parameter>, or if the node to
put in is one of this node's ancestors or this node itself.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This might make sense to either use an entity in language snippets, or possibly add a file to XInclude with an URL to a local file? (maybe for translations one want's a fallback that traverses up to directory structure then go down with ../en/reference/dom/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For ext/random, I have this:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed via entity

Comment thread reference/dom/domdocument/adoptnode.xml Outdated
Copy link
Copy Markdown
Member Author

@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.

LGTM! Thank you!

@Girgias Girgias merged commit 7b1704c into php:master Nov 12, 2023
@Girgias Girgias deleted the stub-8.3-dom branch November 12, 2023 13:40
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.

4 participants