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 qps-ploc generation for store translations #17526

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jul 8, 2024

  • Modified Generate-PseudoLocalizations.ps1 to find the .xml files.
    (As opposed to .resw for the other translations.)
  • Added support for the new format by adding new XPath expressions,
    and stripping comments/attributes as needed.
  • Fixed PreserveWhitespace during XML loading.
  • Fixed compliance with PowerShell's strict mode.

Validation Steps Performed

Ran it locally and compared the results. ✅

@lhecker lhecker added the Area-Build Issues pertaining to the build system, CI, infrastructure, meta label Jul 8, 2024
@lhecker lhecker requested a review from DHowett July 8, 2024 14:44
Get-ChildItem -Recurse -Filter *.resw
| Where-Object { $_.Directory.Name.StartsWith("qps-ploc") }
Get-ChildItem -Recurse -Directory -Filter qps-ploc*
| Get-ChildItem -Include *.resw,*.xml
Copy link
Member Author

Choose a reason for hiding this comment

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

$newValue += $padding.Substring(0, $paddingLength)
$value += $padding.Substring(0, $paddingLength)
return $value
}
Copy link
Member Author

@lhecker lhecker Jul 8, 2024

Choose a reason for hiding this comment

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

Both functions are identical to the code that was previously in the loop below.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

thanks for doing this!

$entry.value = $newValue
}
} elseif ($path.EndsWith(".xml")) {
foreach ($entry in $content.DocumentElement.SelectNodes('//*[@_locID]/text()')) {
Copy link
Member

Choose a reason for hiding this comment

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

technically Locked exists in this format too; do we not have any?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, we actually do.

57: <!-- _locComment_text="{MaxLength=1500} {Locked=__VERSION_NUMBER__} App Release Note" -->Version __VERSION_NUMBER

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah f... Let me try to fix that.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

since we have locked strings

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 8, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 8, 2024
@DHowett DHowett merged commit 5bbd905 into main Jul 8, 2024
12 of 17 checks passed
@DHowett DHowett deleted the dev/lhecker/ploc branch July 8, 2024 22:26
DHowett pushed a commit that referenced this pull request Jul 8, 2024
This fixes some more issues not properly covered by #17526:
* Fixed `_locComment_text` comments being effectively ignored.
* Fixed line splitting of comments (CRLF vs LF).
* Fixed BOM suppression.
* Fixed support for having multiple `{Locked=...}` comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants