php / doc-base

Tools for the PHP documentation
Other
343 stars 85 forks source link

[WIP] Add RFC skeleton for interfaces #61

Closed mallardduck closed 2 years ago

mallardduck commented 2 years ago

This is a WIP pull request that will have cousin PRs in varying PHP repos.

cmb69 commented 2 years ago

Thank you for the PR!

However, I think we need to fix PhD first to properly support this notation. And that might be ugly. :(

mallardduck commented 2 years ago

It is a little ugly, but I've gotten it working. It's really not that much uglier than the existing PHD code, and the latest scheme change I pushed here allows it be done in a way that doesn't touch existing manual pages. So the notation used by things today would still render exactly the same as it does today, to "fix" those pages they would need to use this new notation (which then triggers the new render paths).

There's only one funky bug that i'm looking through - but may take me some time to come back to sorting out. Basically with the notation I've created something is adding white space in the HTML between span tags. This results in gaps between the interface names and the ,. Like so: Screen Shot 2022-04-28 at 5 41 06 PM

Edit: add another screenshot.

Screen Shot 2022-04-28 at 5 41 59 PM

Basically once the line breaks between one span ending and the next begin are removed this should look good and produce results we desire.

mallardduck commented 2 years ago

Think my PR to PHD gets things working right. Which I believe this leaves the next main "issue" to updating the docgen side of things to use the new proposed skeleton shape. However, I do know for sure that the skeleton I proposed technically works with the changes in https://github.com/php/phd/pull/64.

cmb69 commented 2 years ago

Hmm, I'm afraid DocBook does neither define <interfacesysnopsis> nor <interfacesynopsisinfo>. Unless we want to introduce it for our own purposes and further deviate from the standard (what I wouldn't like), we should stick with <classsynopsis> and use the predefined class attribute with values class and interface, respectively. The <classsynopsisinfo> for interfaces likely doesn't need an attribute, as that can be determined from the context.

Anyhow, thank you very much for looking into this! :)

mallardduck commented 2 years ago

@cmb69 - that's actually in the PR as well.

EDIT:

further deviate from the standard (what I wouldn't like)

I guess I'll just scrape this whole effort then. As I can barely reason my way thru how PHD executes linearly. So I just opted to the "Add more elements" to delineate how PHD should render an interface versus a class. To me this seems very reasonable as a way to delineate the two concepts further.

At the very least it's a more intuitive option since currently we're calling an Interface a Class in the XML. And then the rendered CSS refers to it as a class while PHD renders the content as an abstract class. All of those confusions seem quite a bit more concerning IMO - especially since those are areas of PHP that are exposed to everyone that uses PHP.

I guess put another way, my goal with these string of patches was to solve a glaring issue. Something to that fits the "80% rule", which would benefit most users with little negative side-effects. I guess it seems while these patches obviously fix the overt issue, I still missed the mark. If this isn't the optimal way to solve this then I really don't think I have the brain power to help here.

cmb69 commented 2 years ago

One of the problems with newly introduced elements is that they won't even pass the validation stage (./configure); we would need to adjust the DTD, and that will make updating to the latest DocBook (we're years behind) even harder, since that now uses RelaxNG (if I remember correctly).

And the required changes are presumably not that hard to do. I mean, changing the skeleton (and the existing docs) is trivial. Any changes to PhD are unlikely to be trivial (that's why I called that "ugly"), but it might not be too hard either. I'll try to have a closer look at https://github.com/php/phd/pull/64.

mallardduck commented 2 years ago

won't even pass the validation stage (./configure); we would need to adjust the DTD

Yeah that's what I meant - I've already updated the DTD. Ensured that the configure can run properly and then worked on making the PHD commit and PR.

The part about updating the latest DocBook I admit I'm fuzzy on, do I need to propagate any changes I made to the DTD somewhere else? I can take a stab at that if directed the right way.

In any case maybe upon further review of the PR to PHD things will seem less ugly and more acceptable. Or hopefully at the very least it will inspire you or someone more experienced on how to accomplish this.


Legitimately, for years I thought DS provided (more) classes (than it does) due to the docs showing them as abstract classes. So I think there's immense value here to correct and prevent future confusion. I'm a product of the Internet when half the answers were "RTFM" still, and one thing that taught me is that telling someone to RTFM only goes as far as the FM is accurate. I'm glad things have gotten a bit more welcoming than those days tho, but still a context/POV I appreciate having.