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

Document named arguments #251

Closed
wants to merge 3 commits into from
Closed

Conversation

ekinhbayar
Copy link
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
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
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
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
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
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
Contributor Author

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
Contributor

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
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
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.

language/functions.xml Show resolved Hide resolved
</example>

<para>
You can combine named arguments with positional arguments. In this case,
Copy link
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
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
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
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
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.

None yet

3 participants