Add "Do not translate" and "XML Broken" sections to revcheck - #334
Add "Do not translate" and "XML Broken" sections to revcheck#334lacatoire wants to merge 1 commit into
Conversation
Both were already being computed and then dropped. Files marked <?do-not-translate?> were detected and skipped without a trace, and libxml errors were collected by XmlUtil then cleared without ever being read. They are now listed, in the file summary and in a section of their own. Telling a real error apart from an unresolved entity was already solved in scripts/broken.php. That logic moves to a shared XmlErrorFilter, so both callers follow the same rule. Entity files are skipped, as DTD fragments never parse as standalone XML. Files marked do not translate are listed but kept out of the totals, otherwise every translation would see its completion rate drop for files it is not expected to have. The files table gains an xmlError column, so the web report can say what is wrong, not only which file. An empty file raised a ValueError in XmlUtil::loadText() and aborted the run. It is now reported as any other broken file.
alfsb
left a comment
There was a problem hiding this comment.
This is impressive work. I will do some tests, and then merge in about a week. Further reviews and comments welcome.
The only comment is about the comment on revtag failing when XML is detected broken. This is true on empty files, but the (almost now extinct) real broken XML files can have valid revtags (as they are text searched) even if XML is misaligned or otherwise invalid.
| // Only .xml files are checked. Entity files are DTD fragments, | ||
| // never standalone XML, so they always fail to parse as such. | ||
|
|
||
| if ( str_ends_with( $entry->file , '.xml' ) == false ) |
There was a problem hiding this comment.
There are some .ent files that are in fact XML, and some .ent files that are DTD parts. I'm trying to slowly fix this, to have XML in .ent and .xml, and to move DTD entity files as .dtd, or erase them entirely, so in the end all .xml and .ent files will be XML.
Here, the test is about not adding errors for these .ent DTD files. Assume everything is XML in principle, and exclude the known case: files not .xml, that also contains <!ENTITY.
The file is "hot" in OS cache, so reloading these few not .xml cases here to check contents will cause no visible regressions.
|
Tested. Detected one change in doc-ru ("Up to date files" + 1), but otherwise ok. Plan to merge this in one week. Further reviews and comments welcome. |
alfsb
left a comment
There was a problem hiding this comment.
Difference in doc-ru is a change from TranslatedOk into XmlBroken transition, so it is expected.
Both were already being computed and then dropped: files marked
<?do-not-translate?>were detected and skipped without a trace, and libxml errors were collected byXmlUtilthen cleared without ever being read. This lists them, in the file summary and in a section of their own.Telling a real error apart from an unresolved entity was already solved in
scripts/broken.php; that logic moves to a sharedXmlErrorFilterso both callers follow the same rule. Entity files are skipped, as DTD fragments never parse as standalone XML.Files marked do not translate are listed but kept out of the totals, so no translation sees its completion rate drop for files it is not expected to have. The
filestable gains anxmlErrorcolumn, so the web report can say what is wrong and not only which file.An empty file raised a
ValueErrorinXmlUtil::loadText()and aborted the run. It is now reported as any other broken file.Item "Do not translate and XML Broken sections in revcheck" of #199.
Web side, needed for the statuses to show up on doc.php.net: php/web-doc#68
Checked with php 8.4:
scripts/broken.phpoutput unchanged on doc-fr, doc-es, doc-en, doc-ru, doc-de, doc-it and doc-tr; the doc-fr report identical outside the new sections, totals included; XML detection agreeing withbroken.phpon the same six languages, with the same single finding.