Skip to content
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

Support variadic parameters – take 2 #29

Closed
wants to merge 2 commits into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Sep 13, 2020

So far, PhD does not support variadic parameters. Instead the manual
follows the convention to use ... as parameter name, which is then
rendered as $... in the docs. However, as of PHP 5.6.0 variadic
parameters are supported using the syntax ...$args; therefore, we are
going to support this notation for PhD.

While DocBook supports a <varargs> element, this cannot be used in
combination with named parameters. Therefore, we use the rep
attribute of the <methodparam> element to designate variadic
parameters.


What I don't like about this solution is the semantic mismatch. Consider, e.g. array_merge(array ...$arrays); it is not actually $arrays which is repeatable, but rather $array. Maybe I'm too nit-picky here.

TODO:

  • adapt non XHTML based formats
  • fix parameter linking for web-php

So far, PhD does not support variadic parameters.  Instead the manual
follows the convention to use `...` as parameter name, which is then
rendered as `$...` in the docs.  However, as of PHP 5.6.0 variadic
parameters are supported using the syntax `...$args`; therefore, we are
going to support this notation for PhD.

While DocBook supports a `<varargs>` element, this cannot be used in
combination with named parameters.  Therefore, we use the `rep`
attribute of the `<methodparam>` element to designate variadic
parameters.
@cmb69 cmb69 mentioned this pull request Sep 13, 2020
@cmb69
Copy link
Member Author

cmb69 commented Sep 13, 2020

Ugh, the PDF renderer seems to rely on PECL/haru, which is abandoned; latest release almost 8 years ago, and unlikely to be compatible with PHP 7.

PS: and there is also php/web-php@beaf588

@cmb69
Copy link
Member Author

cmb69 commented Sep 13, 2020

Also, if we go this way, should rep="repeat" replace choice="opt", or should it be used in addition?

@nikic
Copy link
Member

nikic commented Nov 2, 2020

fix parameter linking for web-php

What's the problem there?

@cmb69
Copy link
Member Author

cmb69 commented Nov 2, 2020

That should be solved with php/web-php#349.

$this->params["ellipsis"] = '...';
} else {
$this->params["ellipsis"] = '';
}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we set params["ellipsis"] here rather than handling it directly in format_methodparam_parameter, like the role is?

Copy link
Member Author

Choose a reason for hiding this comment

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

role="reference" is an attribute of <parameter>, but rep="repeat" is an attribute of <methodparam>.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, duh.

@cmb69
Copy link
Member Author

cmb69 commented Nov 2, 2020

Applied as d59932c.

@cmb69 cmb69 closed this Nov 2, 2020
@cmb69 cmb69 deleted the cmb/variadics-2 branch November 2, 2020 11:11
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.

2 participants