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

2.1 #124

Open
wants to merge 2 commits into
base: 2.0
Choose a base branch
from
Open

2.1 #124

wants to merge 2 commits into from

Conversation

phpmaster99
Copy link

Fixes for PHP 8

PHP 8's enforcement of the rule that optional parameters cannot precede required parameters in function/method signatures
Fix PHP 8's enforcement of the rule that optional parameters cannot precede required parameters in function/method signatures.
@abiusx
Copy link
Contributor

abiusx commented Apr 20, 2024

How does this fix for PHP 8? Seems like you just reordered args into the functions.

@q-u-o-s-a
Copy link

/**
 * Returns descendants of a node, with their depths in integer
 *
 * @param integer $ID
 * @return array with keys as titles and Title,ID, Depth and Description
 *
 */
function descendants($ID)
{
	$res = $this->{$this->type()}->descendantsConditional("ID=?", false, $ID);
	$out = array();
	if (is_array($res))
		foreach ($res as $v)
			$out[$v['Title']] = $v;
	return $out;
}

should be like this!!

@abiusx
Copy link
Contributor

abiusx commented Jun 24, 2024

Was this the only change that was needed to make this PHP 8 compatible?

Copy link

@q-u-o-s-a q-u-o-s-a left a comment

Choose a reason for hiding this comment

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

should be $res = $this->{$this->type()}->descendantsConditional("ID=?", false, $ID);

@q-u-o-s-a
Copy link

q-u-o-s-a commented Jun 25, 2024

I am using the package in Php 8 (8.0.28 and 8.0.30) and the source is working fine.

In Php 8 optional arguments ($AbsoluteDepths=false,$ConditionString) should be after necessary arguments.
Exenation: https://php.watch/versions/8.0/deprecate-required-param-after-optional

Thats the reason why most of your forks (i checked about 20) are doing it this way.

I really recommand to accept th PR after changing:

$res = $this->{$this->type()}->descendantsConditional("ID=?", false, $ID);

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