Remove synopsis code that is not php

[Wes0617]

public string getErrorMessage ( void ) 

should be converted to

public function getErrorMessage(): string

or even brackets around optional parameters, etc.

The php syntax alone is enough to describe itself, there is no need for stuff that belongs to other languages. In fact, i'm sure that the manual syntax not matching php's, makes thing more difficult for novices.

[morrisonlevi]

We'll have to look at the docbook code but I'm fairly certain it's using semantic markup where the return type must come first. We can transform it to the latter type but I'm pretty sure it needs to remain the same in docbook. This would be a good enhancement to do.

[Sobak]

Actually, it would be very good if we could do this without changing documentation sources. No need to update ~80% files and making trouble for the translations.

@morrisonlevi feel free to assign me, I will look into it. I remember Hannes (PhD's creator) saying it's hard to implement, but it's worth trying.

[Sobak]

Implemented as feature branch in Sobak/phd. Sobak/phd@composer...php-methodsynopsis - two separate commits so it's easier to review.

Original state:

Return type:

Return type + removed brackets around optional parameters:

I wasn't, however, able to remove those spacings. It doesn't require changes in DocBook except of:

• specyfing values for optional parameters - I think it was always desired anyway
• no support for using <void /> as the return type <type>void</type> must be used but I'll think more on that (<void /> can still be used instead of parameters list)

Please note that changes are affecting only PHP Package.

Thoughts?

[morrisonlevi]

What spacing are you talking about getting rid of? The stuff around ( and )?

[Sobak]

Yep. This plus spaces before commas. I thought about using _text method mapped to methodsynopsis in order to remove newlines in HTML output (which then results in those spaces if I'm right) but it affects text inserted directly, not result of parsing nested tags.

[morrisonlevi]

The visual changes look good - I will review the code changes later. Thank you for looking at this!

[Sobak]

You welcome :)

[kadet1090]

I'm hugely in favor of introducing PHP style return type definition but I don't think that removing brackets from optional parameters is good. This is de facto standard across documentation of all languages even if it's not directly reflected by language syntax. It makes easier to spot place where required parameters ends, rather than searching for first occurrence of single =, and I think that many others will think the same.

[morrisonlevi]

It looks as if it always adds the colon whether there is a return type. Are we certain all of them use void or a type there?

[rquadling]

As the return type is optional, we should be following the PHP7 code style of a return type being : <return type hint>.

[Sobak]

Isn't that exactly what is on the screens (and thus implementation)? I don't see a difference

[rquadling]

Ha! I failed to type the rest of my message. It was in my head. Just never made it to my fingers.

As the return type is optional, we should be following the PHP7 code style of a return type being : <return type hint>. But if one isn't defined, it may not be safe to assume that : void is the return type and so only render what is documented.

Having said that, a <methodsynopsis> without a return type tag COULD be considered an error and an appropriate warning should be supplied. This presents a useful unit of work for someone to find all the valid return types. If someone hasn't explicitly documented the return type, assuming it is void would need to be something universally accepted and not just inferred by one piece of software.

Consideration of potential union types (rather than dumb mixed) may also be a worthwhile exercise. Generate a warning for mixed return types so they can be mapped to the correct union types (strpos ( string $haystack , mixed $needle [, int $offset = 0 ] ): int|false for example).

But that's a PHP 7.1 syntax and so for PHP < 7.1 the syntax, as a template for new users to write their own code, is wrong. But I can live with that.

Having said that, http://tdg.docbook.org/tdg/5.2/methodsynopsis.html doesn't seem to allow for multiple <types>. So the XML would have to represent the union type itself <type>int | false</type>. Which isn't ideal.

[rquadling]

I've made a suggestion to DocBook to allow a node for multiple types (docbook/docbook#91). We shall see what happens there.

[Sobak]

Nice, thanks for taking the initiative. And good luck, obviously!