Rendering issues for Interfaces that extend another one

[Girgias]

There is a missing space between the name of the interface and the extend keyword.

See https://www.php.net/manual/en/class.throwable.php for an example.

@mallardduck could you have a look considering you made the rendering changes.

[mallardduck]

Would be fixed by php/doc-en#2560

Happening because they aren't using the same docbook markup.

[mallardduck]

🤔 Otherwise if the desire is to implement that in PHD instead - then I think that would require introducing something like format_classsynopsisinfo_ooclass to match the what format_classsynopsisinfo_oointerface does? At least I think that's the direction a fix would need to go via PHD vs adjusting docs.

[Girgias]

So, I had a look at the DTD and at how we are currently marking up our classes/interfaces.

And it doesn't make any sense, I think how we should be doing the markup for an interfaces is as follows:

   <classsynopsis class="interface">
    <oointerface>
     <interfacename>Throwable</interfacename>
    </oointerface>
    
    <oointerface>
     <modifier>extends</modifier>
     <interfacename>Stringable</interfacename>
    </oointerface>
    
    <oointerface>
     <modifier>extends</modifier>
     <interfacename>InterfaceTwo</interfacename>
    </oointerface>

    <classsynopsisinfo role="comment">&Methods;</classsynopsisinfo>
    <xi:include xpointer="xmlns(db=http://docbook.org/ns/docbook) xpointer(id('class.throwable')/db:refentry/db:refsect1[@role='description']/descendant::db:methodsynopsis[@role='Throwable'])">
     <xi:fallback/>
    </xi:include>

    <classsynopsisinfo role="comment">&InheritedMethods;</classsynopsisinfo>
    <xi:include xpointer="xmlns(db=http://docbook.org/ns/docbook) xpointer(id('class.stringable')/db:refentry/db:refsect1[@role='description']/descendant::db:methodsynopsis[@role='Stringable'])">
     <xi:fallback/>
    </xi:include>
   </classsynopsis>

And for a class it should be:

   <classsynopsis class="class">
    <ooclass>
     <classname>SomeClass</classname>
    </ooclass>
    
    <ooclass>
     <modifier>extends</modifier>
     <classname>ExtendingParentClass</classname>
    </ooclass>

    <oointerface>
     <modifier>implements</modifier>
     <interfacename>InterfaceOne</interfacename>
    </oointerface>

    <oointerface>
     <modifier>implements</modifier>
     <interfacename>InterfaceTwo</interfacename>
    </oointerface>

    <classsynopsisinfo role="comment">&Methods;</classsynopsisinfo>
    <xi:include xpointer="xmlns(db=http://docbook.org/ns/docbook) xpointer(id('class.throwable')/db:refentry/db:refsect1[@role='description']/descendant::db:methodsynopsis[@role='Throwable'])">
     <xi:fallback/>
    </xi:include>

    <classsynopsisinfo role="comment">&InheritedMethods;</classsynopsisinfo>
    <xi:include xpointer="xmlns(db=http://docbook.org/ns/docbook) xpointer(id('class.stringable')/db:refentry/db:refsect1[@role='description']/descendant::db:methodsynopsis[@role='Stringable'])">
     <xi:fallback/>
    </xi:include>
   </classsynopsis>

As it doesn't really make any sense to wrap the "definition" of it in a <classsynopsisinfo> tag.

The class attribute should give us the info to put the word class or interface before anything else, and then the only thing that needs to be done is to collapses the lists of interfaces which have the modifier.

Or if that's too hard for PhD (considering I really don't like touching it as I don't understand it) one can just shove the modifier in the first <oointerface> tag and get the processing for free rather than needing to tinker and try to render the <classsynopsisinfo> tag.

[Girgias]

Re: using the class attribute.

That doesn't really work because of the final and abstract modifiers

[mallardduck]

So without any modification this is what the proposed markdown provides.

So to get it close to what the current solution does, we need to address:

• Lack of interface being printed before the object name.
• Multiple extends need to be collapsed and redundant extends replaced by ,
• The local class name needs to be removed from local methods.

I suspect this will lead to a good deal of duplicated code, but I'll give it a shot.

[mallardduck]

@Girgias actually I think your desire to remove classsynopsisinfo will fundamentally break things. It seems like everything PHD does right now requires classes and interfaces to be wrapped that way. Maybe @heiglandreas has a better idea than myself - but to me this looks like more work than it's worth.

The way I see this, it would require both significant changes to PHD and to every subsequent language of PHP docs. Either that, or we make the changes in a fully backwards compatible way and eventually can remove the old deprecated methods? Because by removing that seemingly "extra" classsynopsisinfo really breaks all of PHDs existing logic.

[heiglandreas]

I see where you are comming from, @Girgias: But the oointerface can be part of the classsynopsisinfo as well as the classsynopsis. And yes: Logically it would make more sense in the classsynopsis!

But implementing that in PHD in a BC way is a tad more and will probably consiberably delay the immediate benefit of just getting on.

So I'd leave it as it is right now. And then think about how we can implement a backwards-compatible solution in PHD. And possibly find a way of converting the different languages in an automated way into the new structure.

And then replace the DTD with an XSD 😇?

[Girgias]

@Girgias actually I think your desire to remove classsynopsisinfo will fundamentally break things. It seems like everything PHD does right now requires classes and interfaces to be wrapped that way. Maybe @heiglandreas has a better idea than myself - but to me this looks like more work than it's worth.

The way I see this, it would require both significant changes to PHD and to every subsequent language of PHP docs. Either that, or we make the changes in a fully backwards compatible way and eventually can remove the old deprecated methods? Because by removing that seemingly "extra" classsynopsisinfo really breaks all of PHDs existing logic.

The previous interface changes already required changes to translations, if the translation is up to date they are extremely easy to apply in that one can just play back the doc-en commit and update the EN-Revcheck tag.

I've made a PR to handle this in a BC way in #77 which should fix most issues and in the long run simplify the logic of the renderer. (and also make my life easier for my own docbooks render project I'm working on because PhD is such an ungodly mess).

[heiglandreas]

I was just thinking about ... XSLT ... 🙈

[Girgias]

Wasn't the whole purpose of PhD to stop using XSLT because of how slow it is?

Moreover, I don't think XSLT would solve some of the larger issue the current renderer faces.

[heiglandreas]

Wasn't the whole purpose of PhD to stop using XSLT because of how slow it is?

I scrolled back until 2008 in the history of PhD. So while it might have been true at the time that rendering using XSLT was slow, did anyone check that?

An using XSLT might indeed solve some things as we are using DocBook and there are default renderers for DocBook. So we might actually profit from what other people did.

Yes! I agree! Our Layout looks different! But I would wonder if that is not something that CSS can solve.
Yes! I also agree! That would completely throw our current build system to bits.

Which is why that was a thought! But perhaps one that we should at least think through after 15 years of PhD.

Moreover, I don't think XSLT would solve some of the larger issue the current renderer faces.

That might be! I only thought about it as I'm sure that we aren't the only ones with the issue of interfaces and stuff... And I'm not even thinking about enums right now 😁

[mallardduck]

@Girgias - I think we sorted this issue out with all of our changes to interface rendering. Right?

[kocsismate]

Yeah, issues are fixed via #77 and php/doc-en#2620! There is also a followup (php/doc-en#2611) which still needs to be merged, but this doesn't strictly relate to the issue.