PHP 8.4: Document `StreamBucket` #4297

kocsismate · 2024-12-12T07:38:23Z

Adding docs for the StreamBucket class.

Girgias · 2024-12-12T08:28:34Z

reference/stream/functions/stream-bucket-make-writeable.xml

+    <varlistentry>
+     <term>
+      <property>dataLength</property>
+      (<type>int</type>)
+     </term>
+     <listitem>
+      <para>
+       <parameter>dataLength</parameter> <literal>bucket</literal> The length of the string in the bucket.
+      </para>
+     </listitem>
+    </varlistentry>


Can't we just remove the property description now? I would rather have something along the lines of:

Prior to PHP 8.4.0, an stdClass containing the same properties as StreamBucket was returned.

Then duplicating documentation.

Scream-shot from the current docs:

What is the meaning of data(len) bucket there? This looks like pure noise.

Yeah, good point! I wasn't sure if we want to keep these docs or not, so I left it there. But I agree, these are just noise after adding the StreamBucket page.

Girgias · 2024-12-12T08:29:30Z

reference/stream/streambucket.xml

+  <para>
+   <simplelist>
+    <member><function>stream_bucket_new</function></member>
+    <member><function>stream_bucket_append</function></member>
+    <member><function>stream_bucket_prepend</function></member>
+    <member><function>stream_bucket_make_writeable</function></member>
+   </simplelist>
+  </para>


No useless wrapping para tags

Suggested change

<para>

<simplelist>

<member><function>stream_bucket_new</function></member>

<member><function>stream_bucket_append</function></member>

<member><function>stream_bucket_prepend</function></member>

<member><function>stream_bucket_make_writeable</function></member>

</simplelist>

</para>

<simplelist>

<member><function>stream_bucket_new</function></member>

<member><function>stream_bucket_append</function></member>

<member><function>stream_bucket_prepend</function></member>

<member><function>stream_bucket_make_writeable</function></member>

</simplelist>

Girgias · 2024-12-12T08:29:41Z

reference/stream/streambucket.xml

+ <section role="seealso">
+  &reftitle.seealso;


Indent is off

Girgias · 2024-12-12T08:30:28Z

reference/stream/streambucket.xml

+    <varlistentry xml:id="streambucket.props.bucket">
+     <term>resource <varname>bucket</varname></term>
+     <listitem>
+      <para>A <literal>userfilter.bucket</literal> resource.</para>


Please use simpara now, and use new lines for the descriptions. Apply for the other properties.

Suggested change

<para>A <literal>userfilter.bucket</literal> resource.</para>

<simpara>

A <literal>userfilter.bucket</literal> resource.

</simpara>

What's the difference between the two?

Roughly, simpara is like a HTML p while para is more like a div.

Girgias · 2024-12-12T08:30:41Z

reference/stream/streambucket.xml

+   <para>
+    A stream bucket is a chunk of stream which can be extracted from bucket brigades.
+   </para>


Suggested change

<para>

A stream bucket is a chunk of stream which can be extracted from bucket brigades.

</para>

<simpara>

A stream bucket is a chunk of stream which can be extracted from bucket brigades.

</simpara>

Girgias · 2024-12-12T08:31:49Z

reference/stream/functions/stream-bucket-make-writeable.xml

+ <refsect1 role="changelog">
+  &reftitle.changelog;
+  <informaltable>
+   <tgroup cols="2">
+    <thead>
+     <row>
+      <entry>&Version;</entry>
+      <entry>&Description;</entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry>8.4.0</entry>
+      <entry>
+       This function returns a <classname>StreamBucket</classname> instance now;
+       previously, an <classname>stdClass</classname> was returned.
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </informaltable>
+ </refsect1>


Feels like the changelogs could just XInclude the one from stream_bucket_append() to prevent duplication.

TBH I'm not sure it's worth the effort for only two places to include. 🤔

Xincluding is not really more difficult than copy pasting.

cmb69 · 2024-12-12T11:44:36Z

reference/stream/functions/stream-bucket-make-writeable.xml

+    <varlistentry>
+     <term>
+      <property>dataLength</property>
+      (<type>int</type>)
+     </term>
+     <listitem>
+      <para>
+       <parameter>dataLength</parameter> <literal>bucket</literal> The length of the string in the bucket.
+      </para>
+     </listitem>
+    </varlistentry>


Scream-shot from the current docs:

What is the meaning of data(len) bucket there? This looks like pure noise.

reference/stream/streambucket.xml

Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>

Girgias · 2024-12-13T01:49:46Z

reference/stream/functions/stream-bucket-make-writeable.xml

-     </listitem>
-    </varlistentry>
-   </variablelist>
+   Returns a bucket object or &null;.


When does it return null actually?

As far as I managed to understand https://github.com/php/php-src/blob/d6d78545ea405e89a44445a9a32d77a75fd8bb6b/ext/standard/user_filters.c#L356, it returns null if the brigade is empty (?). The 2nd part of the condition seems to be always non-null (bucket variable).

Yeah, at least under ZendMM, php_stream_bucket_make_writeable() cannot return NULL; that code is from 2003; maybe we should move the php_stream_bucket_make_writeable() inside the if-branch.

Anyhow, reading the manual helps understanding the implementation. E.g. the info on https://www.php.net/manual/en/function.stream-bucket-append.php explains everything; and https://www.php.net/manual/en/function.stream-bucket-make-writeable.php (emphasis mine):

This function is called whenever there is the need to access and operate on the content contains in a brigade.

is also quite entertaining. ;)

Might not make sense to try to figure out this particular return value meaning for now.

kocsismate · 2024-12-19T21:22:50Z

@Girgias Is there anything else I should fix in order to merge this?

Synchronize manual with stubs for PHP 8.4 - part 10

3f8a4bc

kocsismate added the method synopsis label Dec 12, 2024

kocsismate requested review from cmb69 and Girgias December 12, 2024 07:38

Girgias requested changes Dec 12, 2024

View reviewed changes

Girgias changed the title ~~Synchronize manual with stubs for PHP 8.4 - part 10~~ PHP 8.4: Document StreamBucket Dec 12, 2024

Girgias mentioned this pull request Dec 12, 2024

PHP 8.4 documentation tracker #3872

Open

Girgias added this to the PHP 8.4 milestone Dec 12, 2024

cmb69 reviewed Dec 12, 2024

View reviewed changes

kocsismate and others added 3 commits December 12, 2024 22:34

Address review comments

dae9b52

Update reference/stream/streambucket.xml

3a5d6af

Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>

Remove <para> tag

a663438

Girgias reviewed Dec 13, 2024

View reviewed changes

Use entities

2f2fd66

Girgias approved these changes Dec 20, 2024

View reviewed changes

Girgias merged commit 32caa89 into php:master Dec 20, 2024
2 checks passed

kocsismate deleted the php84-sync-10 branch December 20, 2024 13:49

siwa32 mentioned this pull request Dec 21, 2024

PHP 8.4 マニュアル翻訳状況 php/doc-ja#150

Open

PHP 8.4: Document StreamBucket #4297

PHP 8.4: Document StreamBucket #4297

Uh oh!

Conversation

kocsismate commented Dec 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kocsismate commented Dec 19, 2024

Uh oh!

Uh oh!

Uh oh!

PHP 8.4: Document `StreamBucket` #4297

PHP 8.4: Document `StreamBucket` #4297