Skip to content

Conversation

@Artoria2e5
Copy link

@Artoria2e5 Artoria2e5 commented Sep 22, 2024

A lot of things changed here:

  • Rename zh to zh_CN to make room for zh_TW.
  • Improve existing zh_CN translation.
  • Make a new zh_TW thing.
  • Change perl code to add zh_TW.
  • Change perl code to handle other things browsers ask for: zh-HK, MO, and the lesser-known Chinese variants.

The most dubious thing here is the perl code. Please, tell me if it's working at all.

@Artoria2e5
Copy link
Author

Conflicts already?!

* Use <u> instead of <i>, because Chinese italics is woe
We are going to have Taiwanese and Hong Kongese friends eventually, so
let's be more specific.
Mostly issues with punctuation and how line breaks create spaces, but
some sentences were reworded for clarity too.
I am very unsure about this new logic. Perl, welp.
... using the two commands I used to use half a decade ago:

* opencc -c s2twp
* sed -e 's/“/「/g' -e 's/”/」/g' -e 's/‘/『/g' -e 's/’/』/g'
@Artoria2e5
Copy link
Author

Artoria2e5 commented Mar 6, 2025

I have to say the merge of @libkoi's PR was less than fortunate. Anyways, I took some of libkoi's translation during conflict resolution, but mostly kept mine -- partly because the file move confuses git's three way display on some files, so I am not even seeing the changed text...

In other words... @abh good to merge again, please review.

@abh abh requested a review from Copilot June 5, 2025 23:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR focuses on improving Chinese-language support by updating language identifiers, translations, and alias handling while also including several documentation updates. Key changes include:

  • Renaming the key "zh" to "zh_CN" and adding "zh_TW" in the language mappings.
  • Updating translation files (.po) to modify the formatting tags and wording.
  • Revising several documentation pages in both Traditional and Simplified Chinese.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

File Description
lib/NTPPool/Control.pm Updated language mappings and enhanced alias resolution in Perl code.
i18n/zh_TW.po and i18n/zh.po Modified translation strings for consistency and formatting.
docs/ntppool/zh_TW/* and docs/ntppool/zh_CN/use.html Updated documentation content in Traditional and Simplified Chinese.
docs/ntppool/zh/use.html Removed outdated Simplified Chinese use guidance.

Comment on lines +247 to +250
return first {
s/-/_/g;
$valid_languages{$_} // $valid_languages{$language_aliases{$_}}
} @languages;
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

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

[nitpick] Using the in-place substitution operator within the 'first' block reduces readability. Consider preprocessing the language list by replacing '-' with '_' before applying the 'first' function to improve clarity.

Suggested change
return first {
s/-/_/g;
$valid_languages{$_} // $valid_languages{$language_aliases{$_}}
} @languages;
my @processed_languages = map { my $lang = $_; $lang =~ s/-/_/g; $lang } @languages;
return first {
$valid_languages{$_} // $valid_languages{$language_aliases{$_}}
} @processed_languages;

Copilot uses AI. Check for mistakes.
@abh
Copy link
Owner

abh commented Jun 5, 2025

@Artoria2e5 sorry to not get on this sooner; I need to review fixing the language codes to not just be the two letter ones.

There's CDN configuration that only supports two letter codes currently, so I have to work through that and see if I can work through it so the "simple" languages where the two letter code is fine can continue to work as-is.

Repository owner deleted a comment from Copilot AI Jun 6, 2025
@abh abh force-pushed the main branch 3 times, most recently from b215a04 to 1ab7f20 Compare July 25, 2025 03:35
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.

2 participants