Skip to content

Make the usage of the role attribute more clear #9901

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

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

kocsismate
Copy link
Member

@kocsismate kocsismate commented Nov 7, 2022

Currently, the role attribute is used to differentiate OO and procedural "aliases" when they are documented on the same page (e.g. https://www.php.net/manual/en/datetime.diff.php). However, these function-method counterparts are not always aliases in fact according to the stubs (#9491). That's why sometimes the usage of the role attribute is ambiguous, thus syncing the manual with the stubs results in false positive diffs.

This PR fixes the problem by a very obtrusive way: by changing the value of all role="oop" attributes to the actual class name, like role="DateTime", and by getting rid of all role="procedural" attributes as they became unnecessary. This way, class synopsis pages can clearly reference methods, and skip functions.

By having this PR, we can get rid of quite a few false positive diff between the output of gen_stub.php and the manual, most notably in ext/mysqli, ext/date and ext/curl. On the other hand, it will take a lot of followup changes in the manual to actually get rid of the diff. :(

@Girgias
Copy link
Member

Girgias commented Nov 9, 2022

I think in principle this makes sense. Although ideally we wouldn't be using <methodsynopsis> for functions, but that ship has sailed unless we want to go through a massive maintenance burden. So this sounds like an OK compromise.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Yeah, what @Girgias said. We should update the skeletons and the docgen templates accordingly.

@kocsismate kocsismate changed the title Make the role attribute more useful Make the usage of the role attribute more clear Nov 10, 2022
@kocsismate kocsismate merged commit 93605f2 into php:master Nov 10, 2022
@kocsismate kocsismate deleted the manual-role branch November 10, 2022 07:35
kocsismate added a commit to kocsismate/doc-en that referenced this pull request Nov 10, 2022
kocsismate added a commit to kocsismate/doc-en that referenced this pull request Nov 10, 2022
kocsismate added a commit to kocsismate/doc-en that referenced this pull request Nov 10, 2022
kocsismate added a commit to php/doc-en that referenced this pull request Dec 11, 2022
kocsismate added a commit to php/doc-en that referenced this pull request Dec 11, 2022
kocsismate added a commit to kocsismate/doc-en that referenced this pull request Dec 11, 2022
kocsismate added a commit to php/doc-en that referenced this pull request Dec 11, 2022
claudepache pushed a commit to claudepache/php-doc-en that referenced this pull request Jun 1, 2023
claudepache pushed a commit to claudepache/php-doc-en that referenced this pull request Jun 1, 2023
claudepache pushed a commit to claudepache/php-doc-en that referenced this pull request Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants