Skip to content

Document named arguments#251

Closed
ekinhbayar wants to merge 3 commits intophp:masterfrom
ekinhbayar:named-arguments
Closed

Document named arguments#251
ekinhbayar wants to merge 3 commits intophp:masterfrom
ekinhbayar:named-arguments

Conversation

@ekinhbayar
Copy link
Copy Markdown
Contributor

Initial draft for documenting named arguments on the functions page.

Feedback is most welcomed! This PR could also use a quick fact-check and I'd appreciate pointing out what's missing that needs documentation.

This PR also documents deprecation of optional args after mandatory ones.

Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Looks good, other than the mix of parameter and arguments, which we might need to figure out when to use which in what context and try to normalize this in the whole docs :(

<para>
As of PHP 8.0.0, passing optional arguments after mandatory arguments
is deprecated. This can generally be resolved by dropping the default value.
One exception to this rule are parameters of the form
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
One exception to this rule are parameters of the form
One exception to this rule are arguments of the form

To keep this consistent with the beginning of the paragraph?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So I might have been getting the correctness wrong looking at: #241 (comment), so maybe ignore these comments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In this specific example, I think parameters is the correct wording indeed since it's about the function signature which is the place to use the word parameter, though I wouldn't be surprised if I had gotten it wrong elsewhere in the PR :-)

As of PHP 8.0.0, passing optional arguments after mandatory arguments
is deprecated. This can generally be resolved by dropping the default value.
One exception to this rule are parameters of the form
Type $param = &null;, where the &null; default makes the type implicitly
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Type $param = &null;, where the &null; default makes the type implicitly
<code>Type $param = &null;</code>, where the &null; default makes the type implicitly

Maybe? @cmb69 might want to confirm is that would work

Copy link
Copy Markdown
Contributor Author

@ekinhbayar ekinhbayar Nov 29, 2020

Choose a reason for hiding this comment

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

I'd be happy to do this, I wasn't sure if that'd undo the use of &null; tbh. I could totally use null directly instead, if that would be the case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems to me that <code><replaceable>Type</replaceable> $param = &null;</code> might be the proper markup, and might render as desired as well (fingers crossed!)

<para>
PHP 8.0.0 introduced named arguments as an extension of the existing
positional parameters. Named arguments allow passing arguments to a
function based on the parameter name, rather than the parameter position.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
function based on the parameter name, rather than the parameter position.
function based on the argument name, rather than the argument position.

Again for consistency with the beginning of the paragraph

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this one is a legit use of argument, as it's again talking about the name vs position of the parameter at the function definition.

</example>

<para>
You can combine named arguments with positional arguments. In this case,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
You can combine named arguments with positional arguments. In this case,
Named arguments can be combined with positional arguments. In this case,

Usage of "you" in the docs is to be limited to the tutorial section for the most part

</para>

<example>
<title>Same as above example with a different order of parameters</title>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<title>Same as above example with a different order of parameters</title>
<title>Same example as above with a different order of parameters</title>

@ekinhbayar
Copy link
Copy Markdown
Contributor Author

@Girgias Feel free to create an issue for argument vs parameter usage throughout the docs and even assign it to me. I can work on this over time in a single PR. Happy to make sure we keep this consistent across the docs :-) I'll commit the other changes shortly.

Also documents deprecation of optional args after mandatory ones
@ekinhbayar
Copy link
Copy Markdown
Contributor Author

ekinhbayar commented Nov 30, 2020

Yup, by the looks of the failure, I do need to use null explicitly in the code block. Edit: @cmb69, I juuust saw your comment :-) Would you like me to replace it with that? Before this last commit I searched for &null;</code> in the codebase and couldn't find any occurrence. The build seem to have passed with the direct use of null, however, I'm fine trying out (and potentially breaking build) again :-)

@ekinhbayar
Copy link
Copy Markdown
Contributor Author

@cmb69 @Girgias, is there anything I can do here to help closing/completing this one?

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.

3 participants