Command line tools for XML sync testing between languages #222
Conversation
scripts/translation/qaxml-pi.php
Outdated
| if ( implode( "\n" , $s ) == implode( "\n" , $t ) ) | ||
| continue; | ||
|
|
||
| $sideCount = array(); |
There was a problem hiding this comment.
| $sideCount = array(); | |
| $sideCount = []; |
There was a problem hiding this comment.
Replace all array() usage by [] on new code.
scripts/translation/qaxml-pi.php
Outdated
|
|
||
| function extractPiData( array $list ) | ||
| { | ||
| $ret = array(); |
There was a problem hiding this comment.
| $ret = array(); | |
| $ret = []; |
| foreach( $s as $v ) | ||
| $sideCount[$v] = [ 0 , 0 ]; | ||
| foreach( $t as $v ) |
There was a problem hiding this comment.
I don't understand this coding style. Usually it would be something like:
| foreach( $s as $v ) | |
| $sideCount[$v] = [ 0 , 0 ]; | |
| foreach( $t as $v ) | |
| foreach ($s as $v) | |
| $sideCount[$v] = [ 0 , 0 ]; | |
| foreach ($t as $v) |
| continue; | ||
|
|
||
| print "# qaxml.r: $target\n"; | ||
| foreach( $revtag->errors as $error ) |
scripts/translation/qaxml-tags.php
Outdated
| if ( implode( "\n" , $s ) == implode( "\n" , $t ) ) | ||
| continue; | ||
|
|
||
| $sideCount = array(); |
There was a problem hiding this comment.
| $sideCount = array(); | |
| $sideCount = []; |
scripts/translation/qaxml-tags.php
Outdated
|
|
||
| function extractTagsInnerText( array $nodes , array $tags ) | ||
| { | ||
| $ret = array(); |
There was a problem hiding this comment.
| $ret = array(); | |
| $ret = []; |
| if ( in_array( $tag , $tags ) == false ) | ||
| continue; | ||
| $text = $node->textContent; | ||
| while( true ) |
There was a problem hiding this comment.
Similarly CS which I don't understand.
scripts/translation/qaxml-tags.php
Outdated
|
|
||
| function extractTagsInnerXmls( array $nodes , array $tags ) | ||
| { | ||
| $ret = array(); |
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
Why?
| } | |
| else | |
| { | |
| } else { |
scripts/translation/qaxml-tags.php
Outdated
|
|
||
| function collectTagLines( string $file , string $tag ) | ||
| { | ||
| $ret = array(); |
scripts/translation/qaxml-pi.php
Outdated
| $s = extractPiData( $s ); | ||
| $t = extractPiData( $t ); | ||
|
|
||
| if ( implode( "\n" , $s ) == implode( "\n" , $t ) ) |
There was a problem hiding this comment.
Shouldn't this be ===? I think it would make more sense.
There was a problem hiding this comment.
I could do the change, but as they are direct returns of identical called function (implode), the only case where it would compare equals with different types are false like values, basically empty strings and empty arrays. And in this case, they comparing equals is correct, as this is a fast test for "same" result on each side.
There was a problem hiding this comment.
Different ordered elements are what I was thinking. If there's no need for the elements to be in different orders then please use strict comparison.
scripts/translation/qaxml-pi.php
Outdated
| $target = $file->targetDir . '/' . $file->file; | ||
| $output = new OutputBuffer( "# qaxml.p" , $target , $ignore ); | ||
|
|
||
| [ $s , $_ , $_ ] = XmlFrag::loadXmlFragmentFile( $source ); |
There was a problem hiding this comment.
Why are you reusing the variable $_ twice? It's just going to overwrite it. And I don't think you are actually using this variable anywhere so why assign it at all?
There was a problem hiding this comment.
The function loadXmlFragmentFile() has 3 returns, but here the code is only interested on first return, so only one is "named" and all other cases are "discarded".
Replacing $_ by _ causes PHP Fatal error: Assignments can only happen to writable values.
And replacing [ $s , $_ , $_ ] by $s causes PHP Fatal error: Uncaught TypeError: XmlFrag::listNodes(): Argument #1 ($node) must be of type DOMNode, array given.
So I choose a repetitive and uninteresting name, that causes overwrite of variables that need to be positioned, but are unused.
There was a problem hiding this comment.
Yeah, but you don't have to assign it at all. [$source] should do fine.
| $s = XmlFrag::listNodes( $s , XML_PI_NODE ); | ||
| $t = XmlFrag::listNodes( $t , XML_PI_NODE ); |
There was a problem hiding this comment.
| $s = XmlFrag::listNodes( $s , XML_PI_NODE ); | |
| $t = XmlFrag::listNodes( $t , XML_PI_NODE ); | |
| $s = XmlFrag::listNodes( $s , XML_PI_NODE ); | |
| $t = XmlFrag::listNodes( $t , XML_PI_NODE ); |
| $s = XmlFrag::listNodes( $s , XML_PI_NODE ); | |
| $t = XmlFrag::listNodes( $t , XML_PI_NODE ); | |
| $source = XmlFrag::listNodes( $source , XML_PI_NODE ); | |
| $target = XmlFrag::listNodes( $target , XML_PI_NODE ); |
Please don't use single-character variable names.
There was a problem hiding this comment.
This would need to be also replaced in other parts of code, and will need to be tested. For a future change/rewrite of code.
This PR rewrites the last of old qaxml tools into something that is almost independent of "lib revcheck" code, and adapted to be run in all translations.
After this PR is merged, and new tools announced, the old tools/code removing will be possible. The only dependence renaming are
/lib/RevtagParser.php, that is a isolated file, andSyncFileListcalling intoRevcheckRun, that can be replaced with a simple.xmllisting, as the new tools now inspect all files, and does not filter forTranslatedOkanymore.