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

Warn/show failed XIncludes #227

Merged
merged 2 commits into from
Mar 7, 2025
Merged

Warn/show failed XIncludes #227

merged 2 commits into from
Mar 7, 2025

Conversation

alfsb
Copy link
Member

@alfsb alfsb commented Mar 6, 2025

An effective fix for #224. Will generate around or less than 10 warnings in almost all translations, where XInclude failures are "erasing" parts of manual translations.

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.

MSTM

@alfsb
Copy link
Member Author

alfsb commented Mar 6, 2025

I plan to merge this latter in the day, and new QA tools by next week, as they are somewhat related. qaxml-attributes.php warns about both sides of an XInclude failure.

$explain = false;

$xpath = new DOMXPath( $dom );
$xpath->registerNamespace( "xi" , "http://www.w3.org/2001/XInclude" );
$nodes = $xpath->query( "//xi:include" );
foreach( $nodes as $node )
{
if ( $header == false )
Copy link
Member

Choose a reason for hiding this comment

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

What kind of formatting code style are you following? This line isn't following the same style as the line before it.

Copy link
Member Author

Choose a reason for hiding this comment

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

In general, keyword space parenthesis, and spaces around parenthesis and operators. No space before parenthesis of functions or constructors. In this case, the line above that is wrongly formatted, but as it was not changed, I minimized the changes on the diff, not fixing it.

@@ -1001,13 +1001,21 @@ function xinclude_residual( DOMDocument $dom )
// XInclude failures are soft errors on translations, so remove
// residual XInclude tags on translations to keep them building.

$header = false;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe $hasHeader or $needsHeader. What does this variable represent?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was more in the sense of $headerDisplayed / printed / showed. A flag to print and not duplicated the header outpout.

Copy link

Choose a reason for hiding this comment

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

but it not being updated
it should be an array

@alfsb alfsb merged commit 3ab4fe9 into php:master Mar 7, 2025
12 checks passed
@alfsb alfsb deleted the qax8 branch March 7, 2025 11:34
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.

4 participants