Convert from XML::Simple to XML::LibXML #3558
Conversation
| #next MESSAGE if $type eq 'unfinished'; # don't skip unfinished strings, as they may still get used | ||
|
|
||
| push @{$keys{en}{$contextname}{uc $1}}, $message->{source} unless $doneeng; | ||
| # skip messages without an accelerator key |
There was a problem hiding this comment.
OK, we're still skipping even if the translation has an accelerator but the English doesn't.
Shouldn't we
- Check that both English and translation have accelerators or both don't (for documentation consistency) -- or is this done elsewhere?
- Then skip if both don't
If the "both the same" is done elsewhere, this is okay -- but I'd suggest mentioning that in the comment.
There was a problem hiding this comment.
OK, we're still skipping even if the translation has an accelerator but the English doesn't.
I'll check, but I think that's the same with the existing tool.
Shouldn't we
- Check that both English and translation have accelerators or both don't (for documentation consistency) -- or is this done elsewhere?
- Then skip if both don't
That is probably a good idea, but out of scope for this PR, which was only concerned with changing XML library.
If the "both the same" is done elsewhere, this is okay -- but I'd suggest mentioning that in the comment.
There are other improvements that could be done to this tool too, but I would do them, and the good suggestions above, separately. At the moment, the output of the tool is just a starting point to aid manual review as part of the release process.
There was a problem hiding this comment.
Well, it does change the flow, which raises the questions over what the old was doing and what the new is doing.
|
Ran on HEAD of softins/jamulus:perl-xml-libxml I'm pretty certain that's right! Not for now but I'll feature request a summary saying "Checked files, found languages with hotkeys" then only emit the "Possible duplicate hotkeys:" if there are any. (Of course, it's so long since I wrote any Perl, I'm tempted to use AI to translate it to something else -- bash with |
You need to cd to the translation directory first: The new version of the tool produces identical output to the old one. The output is simplistic in that it isn't able to take full account of context to know which accelerators actually conflict, and which matches are actually ok because they are in different menus, for example. But overcoming that is a bigger project than just changing XML library.
If rewriting it, it would need to be into Python with nested dicts. But I'm only a beginner with Python, as opposed to being very at home with Perl for 30 years! |
Things like that could also be changed. Ideally all these checks should be run from the repository root and take care of their own internal requirements. Again, not for this PR. |
2 participants
Short description of changes
This converts the Perl script
tools/checkkeys.plfrom using the deprecatedXML::Simpleto usingXML::LibXMLinstead. There are no changes in functionality or behaviour.CHANGELOG: Tools: update checkkeys.pl to use XML::LibXML
Context: Fixes an issue?
XML::Simpleis now deprecated as reported by @pljones on Discord. This is the only tool in the repo that usedXML::Simple.Does this change need documentation? What needs to be documented and how?
No. Internal operation only.
Status of this Pull Request
Ready to merge
What is missing until this pull request can be merged?
Nothing
Checklist