Skip to content

Conversation

@nielsdos
Copy link
Member

@nielsdos nielsdos commented May 7, 2024

Per https://wiki.php.net/rfc/class-naming-acronyms, change the acronyms in ext/dom's new PHP 8.4 classes.

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Thank you for doing the hard work!

}

/** @alias DOM\DOMException */
/** @alias Dom\DOMException */
Copy link
Member

@kocsismate kocsismate May 8, 2024

Choose a reason for hiding this comment

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

Is it intentional that the class name follows the discouraged pattern of uppercased abbreviations? Is it for internal consistency with the existing pattern? Personally, I'd vote for going full steam PascalCase. This applies for other abbreviations like HTTP, XML, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

The official name is DOMException: https://webidl.spec.whatwg.org/#idl-DOMException and so this was left alone.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, I see. Then I guess the same applies to all the other names which were left intact.

}

class DTDNamedNodeMap implements \IteratorAggregate, \Countable
class DtdNamedNodeMap implements \IteratorAggregate, \Countable
Copy link
Member

Choose a reason for hiding this comment

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

I saw a few properties using discouraged names, like namespaceURI. Can these be renamed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same thing here: namespaceURI is defined per spec with that capitalization.

@nielsdos nielsdos merged commit 6e7adb3 into php:master May 9, 2024
xabbuh added a commit to symfony/symfony that referenced this pull request May 15, 2024
… classes (xabbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[VarDumper]  adapt namespace changes for new DOM extension classes

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

related to php/php-src#14171

Commits
-------

7e25a46 adapt namespace changes for new DOM extension classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants