Rewrite type section #1726

Girgias · 2022-07-28T20:35:21Z

To incorporate typing features added in PHP 8.2.0 and also (hopefully) future proof for additional additions.

This is still a bit WIP as I haven't added DNF types but I already wanted to get some eyes on it to gather some opinions.
I've taken a bit of inspiration from the Rust manual: https://doc.rust-lang.org/reference/types.html

iluuu1994

I'm not super familiar with the documentation format so some of these might be wrong (e.g. I'm not sure if <type> does something special).

language/types/declarations.xml

iluuu1994 · 2022-07-29T16:39:22Z

language/types/declarations.xml

+     value returned must be an &instanceof; the same class as the one
+     the method is called in.


Suggested change

value returned must be an &instanceof; the same class as the one

the method is called in.

value returned must be an &instanceof; the same class as the class of the object itself.

Maybe the wording can be improved. But it has nothing to do with the caller, you can call said method outside of the given class hierarchy.

This is the current wording that already exist. I'll think about improving it

Maybe we can clarify with an example. "For example, an object Cat inheriting Animal::getClone(): static must return a Cat, not just any Animal (which would be the behavior of self)."

language/types/declarations.xml

cmb69 · 2022-07-29T17:01:10Z

I'm not sure if <type> does something special

Actually, that is supposed to create a link, but IIRC this is broken for quite a while now. Anyhow, could use XML entities instead.

language/types/declarations.xml

Crell · 2022-07-31T15:56:36Z

language/types/declarations.xml


-  <warning>
+  <para>


I'm not sure about moving this to a list instead of a table. The table seems more readable to me for this, no?

I prefer the list as it allows nesting, also the table currently has no real grouping (and I don't know how to make it sensible) + a version release column which acts a bit weirdly live (might be some CSS issue)

language/types/declarations.xml

Crell · 2022-07-31T16:48:44Z

language/types/declarations.xml

-  <warning>
+  <para>
+   The list of base types is:
+   <itemizedlist>


Whether it's a list or stays as a table, each item should deep-link to the spot in the page where it's described. (Same for the other index lists in the page.)

Crell · 2022-07-31T16:51:36Z

Glancing over the section... I barely understand why this page is what it is and where it is. Shouldn't a description of different types come way earlier in the section? It's basically at the end now. Is the whole Types section due for an overhaul?

Girgias · 2022-07-31T17:18:06Z

Glancing over the section... I barely understand why this page is what it is and where it is. Shouldn't a description of different types come way earlier in the section? It's basically at the end now. Is the whole Types section due for an overhaul?

Yes, probably, I wrote this page back when PHP 8.0 got released as the information about type declarations where scattered in loads of different places. So this was done to centralise it.

Girgias · 2022-07-31T17:18:27Z

I'm not sure if <type> does something special

Actually, that is supposed to create a link, but IIRC this is broken for quite a while now. Anyhow, could use XML entities instead.

php/phd#33 is meant to fix it, might need to go back to it

cmb69

It seems to me we're mixing "types" and "type declarations", but we should better not. I think it's better to move much of what we have on the type declaration page to the introduction page, and to have only info about type declarations on the former (general info about type declarations, and the special declaration types like self).

Girgias · 2022-09-27T13:19:56Z

I don't really think I've addressed your comment @cmb69, but I'm still really blanking on how to break the relevant bits away, as I wrote it all from writing a type declaration perspective :-/

Girgias · 2022-11-07T04:11:47Z

Okay, I've spent some time again rewritting this, and would love to get your comments and opinions on this.

cmb69

Thank you! This is quite something.

language/types.xml

language/types/type-system.xml

language/types/never.xml

cmb69 · 2022-11-22T16:12:13Z

language/types/declarations.xml

+
+  <sect3 xml:id="language.types.declarations.base.scalar">
+   <title>Scalar types</title>
+   <warning>


This is a bit strange now; previously, there was a list of types, and the section referred to that ("above"). Now, there is only a warning without any general info about the "scalar types". The situation is similar for other sections in this file. Maybe we should link to the general descriptions at the beginning of each <sect3>, or have some general info at the beginning of this page.

language/types/declarations.xml

cmb69 · 2022-11-22T16:24:35Z

language/types/declarations.xml

-   </example>
-
-   <example>
-    <title>Catching <classname>TypeError</classname></title>


Thanks for removing this! We should probably clarify elsewhere that catching Errors is usually not supposed to be done.

language/types/type-juggling.xml

To incorporate typing features added in PHP 8.2.0 and also (hopefully) future proof for additionnal additions

Introduce individual pages for all types Add a type system page which describes the type system in agnostic way to userland type declaration Convert the userland type declaration page to just document neessary info related to writing such declarations Move strict_type coercion handling to the type juggling page

Co-authored-by: Christoph M. Becker <[email protected]>

Girgias · 2022-11-23T17:02:13Z

Okay, I've only performed limited changes to try to address all the review comments. Hopefully this is now good to land and be stabbed by the pitchforks of translators.

@saundefined @mumumu what are your opinions of this as the translators of the 2 other main up to date languages?

saundefined · 2022-11-24T10:33:17Z

@saundefined @mumumu what are your opinions of this as the translators of the 2 other main up to date languages?

LGTM, thanks!

mumumu · 2022-11-24T10:35:15Z

@Girgias Personally, I don't have a problem with it. This PR change is not mahoosive than I expected.

cmb69

Thank you!

language/types.xml

Co-authored-by: Christoph M. Becker <[email protected]>

kitrio · 2022-11-30T18:24:50Z

I found some typo
File language/types/type-system.xml
line 48 <simpara><type>float</type> string</simpara>
it seems like typo
because line 45 already write float <simpara><type>float</type> type</simpara>

line 48 <simpara><type>string</type> type</simpara>
Is this right?

Thank you.

cmb69 · 2022-11-30T18:57:52Z

Thanks for reporting, @kitrio! Should be fixed with 23cda46.

This commit rewrites the whole type section to (hopefully) be better structured and future proof for further additions to the type system. * Each type now gets their individual page instead of being shoved in the type declaration page. * A type system page is added which describes PHP's type system, regardless if it is possible to declare the type in userland or not. Therefore, the type declaration page only has information related to writing type declarations in userland. * The description of strict_type and the type coercion is moved into the type juggling page. * Remove outdated information in string implementation section * Add paragraph about using non string in string context can throw Co-authored-by: Christoph M. Becker <[email protected]>

Girgias requested review from Crell, iluuu1994 and cmb69 July 28, 2022 20:35

Girgias force-pushed the rewrite-type-declaration-page branch from 49a0464 to bceb622 Compare July 28, 2022 20:40

iluuu1994 reviewed Jul 29, 2022

View reviewed changes

Crell reviewed Jul 31, 2022

View reviewed changes

language/types/declarations.xml Outdated Show resolved Hide resolved

Crell reviewed Jul 31, 2022

View reviewed changes

language/types/declarations.xml Outdated Show resolved Hide resolved

Crell reviewed Jul 31, 2022

View reviewed changes

language/types/declarations.xml Outdated Show resolved Hide resolved

Crell reviewed Jul 31, 2022

View reviewed changes

language/types/declarations.xml Outdated Show resolved Hide resolved

Crell reviewed Jul 31, 2022

View reviewed changes

language/types/declarations.xml Outdated Show resolved Hide resolved

Crell reviewed Jul 31, 2022

View reviewed changes

Girgias mentioned this pull request Sep 7, 2022

PHP 8.2 documentation tracker #1803

Closed

Girgias added this to the PHP 8.2 milestone Sep 26, 2022

Girgias force-pushed the rewrite-type-declaration-page branch 2 times, most recently from 6327d5b to 9875534 Compare September 26, 2022 10:58

cmb69 reviewed Sep 26, 2022

View reviewed changes

Girgias force-pushed the rewrite-type-declaration-page branch 2 times, most recently from 2e877f4 to f0ca282 Compare September 27, 2022 13:18

Girgias changed the title ~~Rewrite type declaration page~~ Rewrite type section Sep 27, 2022

Girgias force-pushed the rewrite-type-declaration-page branch 2 times, most recently from 8dd73ca to c31cdf1 Compare November 7, 2022 04:10

iluuu1994 self-requested a review November 7, 2022 07:03

Girgias force-pushed the rewrite-type-declaration-page branch from c31cdf1 to 3b02d68 Compare November 10, 2022 13:46

cmb69 reviewed Nov 22, 2022

View reviewed changes

cmb69 added the mahoosive-change Grab a cup of coffee and dive in. label Nov 22, 2022

Girgias and others added 13 commits November 23, 2022 15:44

Rewrite type declaration page

beba44b

To incorporate typing features added in PHP 8.2.0 and also (hopefully) future proof for additionnal additions

Add DNF type docs

e30b2da

Remove catching type error example

56fbc57

type page amend?

9083d4e

Improve whole type section

75277ac

typo

ff09cff

Add missing word

84a8e0d

Remove outdated information in string implementation section

98ad869

Add paragraph about using non string in string context can throw

6237b1a

Various nits

ac6ffde

Co-authored-by: Christoph M. Becker <[email protected]>

Address review comments

1f7434c

Minor rephrasing to address more review comments

78ba7cd

Girgias force-pushed the rewrite-type-declaration-page branch from 577f6a6 to d5b5c62 Compare November 23, 2022 16:56

Restructure slightly the type declaration page

a2f1915

Girgias force-pushed the rewrite-type-declaration-page branch from d5b5c62 to a2f1915 Compare November 23, 2022 16:59

cmb69 reviewed Nov 24, 2022

View reviewed changes

language/types.xml Outdated Show resolved Hide resolved

language/types.xml Outdated Show resolved Hide resolved

Wording nit

6f5bf27

Co-authored-by: Christoph M. Becker <[email protected]>

Girgias merged commit 161dde4 into php:master Nov 30, 2022

Girgias deleted the rewrite-type-declaration-page branch November 30, 2022 15:39

mumumu mentioned this pull request Oct 1, 2023

Split operator XML file into multiple files #2799

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite type section #1726

Rewrite type section #1726

Girgias commented Jul 28, 2022

iluuu1994 left a comment

iluuu1994 Jul 29, 2022

Girgias Jul 30, 2022

iluuu1994 Jul 30, 2022 •

edited

Loading

cmb69 commented Jul 29, 2022

Crell Jul 31, 2022

Girgias Jul 31, 2022

Crell Jul 31, 2022 •

edited

Loading

Crell commented Jul 31, 2022

Girgias commented Jul 31, 2022

Girgias commented Jul 31, 2022

cmb69 left a comment

Girgias commented Sep 27, 2022

Girgias commented Nov 7, 2022

cmb69 left a comment

cmb69 Nov 22, 2022

cmb69 Nov 22, 2022

Girgias commented Nov 23, 2022

saundefined commented Nov 24, 2022

mumumu commented Nov 24, 2022

cmb69 left a comment

kitrio commented Nov 30, 2022 •

edited

Loading

cmb69 commented Nov 30, 2022

		value returned must be an &instanceof; the same class as the one
		the method is called in.

Rewrite type section #1726

Rewrite type section #1726

Conversation

Girgias commented Jul 28, 2022

iluuu1994 left a comment

Choose a reason for hiding this comment

iluuu1994 Jul 29, 2022

Choose a reason for hiding this comment

Girgias Jul 30, 2022

Choose a reason for hiding this comment

iluuu1994 Jul 30, 2022 • edited Loading

Choose a reason for hiding this comment

cmb69 commented Jul 29, 2022

Crell Jul 31, 2022

Choose a reason for hiding this comment

Girgias Jul 31, 2022

Choose a reason for hiding this comment

Crell Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

Crell commented Jul 31, 2022

Girgias commented Jul 31, 2022

Girgias commented Jul 31, 2022

cmb69 left a comment

Choose a reason for hiding this comment

Girgias commented Sep 27, 2022

Girgias commented Nov 7, 2022

cmb69 left a comment

Choose a reason for hiding this comment

cmb69 Nov 22, 2022

Choose a reason for hiding this comment

cmb69 Nov 22, 2022

Choose a reason for hiding this comment

Girgias commented Nov 23, 2022

saundefined commented Nov 24, 2022

mumumu commented Nov 24, 2022

cmb69 left a comment

Choose a reason for hiding this comment

kitrio commented Nov 30, 2022 • edited Loading

cmb69 commented Nov 30, 2022

iluuu1994 Jul 30, 2022 •

edited

Loading

Crell Jul 31, 2022 •

edited

Loading

kitrio commented Nov 30, 2022 •

edited

Loading