Skip to content

Conversation

@haszi
Copy link
Contributor

@haszi haszi commented Jun 3, 2024

Enable rendering of attributes for classes, exceptions, interfaces, methods and functions. The implementation feels a little off but it seems to work as intended (see attached test attribute_formatting_002.phpt).

I've also found a mistake I've made in the test attribute_formatting_001.phpt where the parameter attribute was outside of the <methodparam> element. I've corrected this by moving the attribute inside the element.

@Girgias
Copy link
Member

Girgias commented Jun 4, 2024

Thank you! I'd like the opinion of @TimWolla and @kocsismate about the generated XHTML

@TimWolla
Copy link
Member

TimWolla commented Jun 9, 2024

I don't have an opinion. I trust that you generate XHTML that looks good 😄

@haszi
Copy link
Contributor Author

haszi commented Jun 9, 2024

Function attributes were not rendering properly and there was no test for them either so I just pushed a commit to fix this.

@haszi
Copy link
Contributor Author

haszi commented Jun 9, 2024

Attributes of class, exception and interface synopses are rendering like this:

class1

@haszi
Copy link
Contributor Author

haszi commented Jun 9, 2024

Attributes of methods

method1

and functions

function1

@TimWolla
Copy link
Member

TimWolla commented Jun 9, 2024

I would prefer having a newline between every attribute as well.

@haszi
Copy link
Contributor Author

haszi commented Jun 9, 2024

I would prefer having a newline between every attribute as well.

Something like in the below screenshots?

Classes/exceptions/interfaces:
class1

Methods:
method1

Functions:
function1

@TimWolla
Copy link
Member

TimWolla commented Jun 9, 2024

Something like in the below screenshots?

Yes, looks great to me. Thank you.

Enable rendering of attributes for classes, exceptions, interfaces, methods and functions.
@haszi haszi force-pushed the Add-non-parameter-attribute-rendering branch from e434d42 to e74ff2e Compare June 9, 2024 20:26
@haszi
Copy link
Contributor Author

haszi commented Jun 9, 2024

I've just pushed the commit to render every attribute on its own line (just like in the last set of screenshots).

@kocsismate
Copy link
Member

Thank you! I'll implement the required gen_stub.php related changes

@kocsismate kocsismate merged commit 472dd3c into php:master Jun 10, 2024
@haszi haszi deleted the Add-non-parameter-attribute-rendering branch June 11, 2024 20:41
@TimWolla
Copy link
Member

Thank you! I'll implement the required gen_stub.php related changes

I believe this never happened, did it?

@cmb69
Copy link
Member

cmb69 commented Oct 22, 2024

In a perfect world, there wouldn't be the need for any changes to gen_stub.php; Docbook should ideally render indepent of the source code formatting. However, we're not living in a perfect world.

Anyhow, do we even have classes etc. or functions with attributes that are documented. I could only find parameters, but their rendering shouldn't be affected anyway.

@TimWolla
Copy link
Member

TimWolla commented Oct 22, 2024

@cmb69

In a perfect world, there wouldn't be the need for any changes to gen_stub.php; Docbook should ideally render indepent of the source code formatting. However, we're not living in a perfect world.

I don't follow. gen_stub.php is used to keep the documentation in sync with the actual code by leveraging the stubs. I'm not sure what this has to do with source code formatting.

Anyhow, do we even have classes etc. or functions with attributes that are documented.

We have: All the attribute classes (and with PHP 8.4 all the deprecated functions and methods). The attribute markup is not yet included in the documentation, because of gen_stub not yet having support for generating it (and because understandably no one bothered to do it by hand).

@cmb69
Copy link
Member

cmb69 commented Oct 22, 2024

I don't follow. gen_stub.php is used to keep the documentation in sync with the actual code by leveraging the stubs. I'm not sure what this has to do with source code formatting.

Ah, sorry, I misunderstood. I thought this was about rendering attributes on separate lines; and that shouldn't require changes to gen_stub.php (but would have had, if gen_stub.php would have already output DocBook with attributes on the same line).

because of gen_stub not yet having support for generating it (and because understandably no one bothered to do it by hand).

And none bothered to improve gen_stub.php either. ;) My stance on automation: great to have it, but if none is available, do it manually, unless that is so much work, that you do the automation upfront. (hmm, guess I just volunteered ;)

@kocsismate
Copy link
Member

I believe this never happened, did it?

Oh, no, I believe I forgot about it at last. :(

@kocsismate
Copy link
Member

Finally I filed php/php-src#16926 :)

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.

5 participants