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

add XSLTProcessor::registerPHPFunctionNS and DOMXPath::registerPhpFunctionNS #4129

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

frederikbosch
Copy link
Contributor

Hopefully this PR is correct.

I was not able to build the docs locally.

frederik@frederik-laptop:/srv/libraries/php-doc-en$ make
docker run --rm -v .:/var/www/en -w /var/www -u 1000:1000 php/doc-en
configure.php: $Id$
PHP version: 8.2.26

Checking for source directory... 
error: Source directory doesn't exist or can't be written to.
make: *** [Makefile:22: xhtml] Error 1

@Girgias
Copy link
Member

Girgias commented Nov 22, 2024

The doc directory must be en and NOT doc-en, the build script is very picky about the name for the folder.

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.

XML markup changes, to mimic in the other new file.

Comment on lines 24 to 27
&reftitle.parameters;
<para>
<variablelist>
<varlistentry>
Copy link
Member

Choose a reason for hiding this comment

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

Remove the wrapping para tag it is unnecessary.

Comment on lines 17 to 19
<para>
This method enables the ability to use a PHP function as a namespaced XPath function inside Xpath expressions.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

New manual code style requires <simpara> instead of <para> when possible.

Comment on lines 47 to 48
The PHP function to call when the Xpath function gets called within the Xpath expression. When a node list is passed
as parameter to the callback, they are arrays containing the matched dom nodes.
Copy link
Member

Choose a reason for hiding this comment

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

Semantic line breaks

Suggested change
The PHP function to call when the Xpath function gets called within the Xpath expression. When a node list is passed
as parameter to the callback, they are arrays containing the matched dom nodes.
The PHP function to call when the Xpath function gets called within the Xpath expression.
When a node list is passed as parameter to the callback,
they are arrays containing the matched dom nodes.

Comment on lines +64 to +66
&reftitle.examples;
<para>
<example>
Copy link
Member

Choose a reason for hiding this comment

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

No need for the wrapping para tag.

Copy link
Member

Choose a reason for hiding this comment

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

Still hasn't been addressed.

Comment on lines 127 to 129
<para>
<simplelist>
<member><methodname>DOMXPath::registerNamespace</methodname></member>
Copy link
Member

Choose a reason for hiding this comment

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

Ditto no wrapping para tag.

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this, apart from Girgias's markup comments I also have 2 content-wise comments, but looks mostly good!

&reftitle.examples;
<para>
<example>
<title><methodname>DOMXPath::registerPHPFunctions</methodname> with <literal>php:functionString</literal></title>
Copy link
Member

Choose a reason for hiding this comment

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

The title of this example is wrong


$xpath = new DOMXPath($doc);

// Register the php: namespace (required)
Copy link
Member

Choose a reason for hiding this comment

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

This comment is wrong

@frederikbosch
Copy link
Contributor Author

Hopefully I have resolved both your remarks.

<methodparam><type>callable</type><parameter>callable</parameter></methodparam>
</methodsynopsis>
<simpara>
This method enables the ability to use a PHP function as a namespaced XPath function inside Xpath expressions.
Copy link
Member

Choose a reason for hiding this comment

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

Missing word? Also line break as we usually try to keep lines under 80 char.

Suggested change
This method enables the ability to use a PHP function as a namespaced XPath function inside Xpath expressions.
This method enables the ability to use a PHP function as a namespaced XPath function
inside a Xpath expressions.

Copy link
Member

@nielsdos nielsdos Nov 25, 2024

Choose a reason for hiding this comment

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

Actually should be "an XPath" instead of "a Xpath" (mind the a vs an, and capitalization)
"an" because it's the pronounciation that counts.

Comment on lines +64 to +66
&reftitle.examples;
<para>
<example>
Copy link
Member

Choose a reason for hiding this comment

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

Still hasn't been addressed.

Comment on lines +29 to +31
<para>
The URI of the namespace.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>
The URI of the namespace.
</para>
<simpara>
The URI of the namespace.
</simpara>

Comment on lines +37 to +39
<para>
The local function name inside the namespace.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>
The local function name inside the namespace.
</para>
<simpara>
The local function name inside the namespace.
</simpara>

Comment on lines +45 to +49
<para>
The PHP function to call when the Xpath function gets called within the Xpath expression.
When a node list is passed as parameter to the callback,
they are arrays containing the matched dom nodes.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>
The PHP function to call when the Xpath function gets called within the Xpath expression.
When a node list is passed as parameter to the callback,
they are arrays containing the matched dom nodes.
</para>
<simpara>
The PHP function to call when the Xpath function gets called within the Xpath expression.
When a node list is passed as parameter to the callback,
they are arrays containing the matched dom nodes.
</simpara>

Comment on lines +19 to +20
</refsect1>
<refsect1 role="parameters">
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
</refsect1>
<refsect1 role="parameters">
</refsect1>
<refsect1 role="parameters">

Comment on lines +34 to +36
<para>
The local function name inside the namespace.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>
The local function name inside the namespace.
</para>
<simpara>
The local function name inside the namespace.
</simpara>

Comment on lines +42 to +46
<para>
The PHP function to call when the XSL function gets called within the stylesheet.
When a node list is passed as parameter to the callback,
the argument becomes an array containing the matched dom nodes.
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>
The PHP function to call when the XSL function gets called within the stylesheet.
When a node list is passed as parameter to the callback,
the argument becomes an array containing the matched dom nodes.
</para>
<simpara>
The PHP function to call when the XSL function gets called within the stylesheet.
When a node list is passed as parameter to the callback,
the argument becomes an array containing the matched dom nodes.
</simpara>

Comment on lines +54 to +56
<para>
&return.void;
</para>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<para>
&return.void;
</para>
<simpara>
&return.void;
</simpara>

Comment on lines +60 to +64
&reftitle.examples;
<para>
<example>
<title>Simple PHP Function call from a stylesheet</title>
<programlisting role="php"><![CDATA[<?php
Copy link
Member

Choose a reason for hiding this comment

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

Ditto, the wrapping <para> tag around the <example> tag is useless, please remove.

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.

3 participants