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

Fix bug #61907: XML Error Codes constants should list integer value #2781

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nielsdos
Copy link
Member

Fixes https://bugs.php.net/bug.php?id=61907.
Stole the table from exif image types page and adapted for ext/xml.

Fixes https://bugs.php.net/bug.php?id=61907.
Stole the table from exif image types page and adapted for ext/xml.
@Girgias
Copy link
Member

Girgias commented Sep 22, 2023

@kocsismate didn't we kind of decide to stop showing unknown constant values (as they come from external libraries)?

@nielsdos
Copy link
Member Author

These aren't really unknown, as they are also emulated by the expat compatibility layer in PHP, and thus must match the library's values.

@Girgias
Copy link
Member

Girgias commented Sep 22, 2023

These aren't really unknown, as they are also emulated by the expat compatibility layer in PHP, and thus must match the library's values.

Right, but they are not defined as such in the stub file? As this is how we generate the constant pages now.

@nielsdos
Copy link
Member Author

These aren't really unknown, as they are also emulated by the expat compatibility layer in PHP, and thus must match the library's values.

Right, but they are not defined as such in the stub file? As this is how we generate the constant pages now.

Ah, well, they're defined UNKNOWN like this:

/**
 * @var int
 * @cvalue XML_ERROR_EXTERNAL_ENTITY_HANDLING
 */
const XML_ERROR_EXTERNAL_ENTITY_HANDLING = UNKNOWN;

@Girgias
Copy link
Member

Girgias commented Sep 22, 2023

We might need to open up a proper discussion on the doc ML about documenting constant values.

I personally don't find it super useful, but that's just my opinion.

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.

2 participants