Fix inconsistencies found by verifyresources

[malxau]

This covers a few different things:

• Adding strings for hardlinks, symlinks and junctions to tr-TR. These aren't localized but it seems better to do that than to have them missing and rely on runtime code handling it.
• Add a pile of JAPANBEGIN strings. This seems a bit strange to me - as far as I can tell, they'd only be referenced if bJapan is TRUE which happens from a runtime check for LANG_JAPANESE. In other words, these strings are to help VerifyResources but are really just wasted space. I'd like to know why Japanese specifically requires extra strings that other languages don't use.
• Adding the "mirror application content" checkbox to de-DE. This is present in other languages as invisible just to satisfy VerifyResources, but not de-DE.
• zh-CN seems to be the first localized resource and forked at a time when the English resource was being changed to remove an accelerator key. Later the resources had SS_NOPREFIX added but still contained the accelerator. ja-JP inherited this from zh-CN, but didn't get SS_NOPREFIX.

[bitigchi]

Hello,

Please find appropriate Turkish translations below:

  IDS_COPYERROR + FUNC_LINK        "Sembolik Bağlantı Oluşturmada Hata"
   IDS_COPYERROR + FUNC_HARD        "Sabit Bağlantı Oluşturmada Hata"
   IDS_COPYERROR + FUNC_JUNC        "Birleşme Noktası Oluşturmada Hata"

   IDS_VERBS + FUNC_LINK    "Dosya Yöneticisi %s sembolik bağlantısını oluşturamıyor: %s"
   IDS_VERBS + FUNC_HARD    "Dosya Yöneticisi %s sabit bağlantısını oluşturamıyor: %s"
   IDS_VERBS + FUNC_JUNC    "Dosya Yöneticisi %s birleşme noktasını oluşturamıyor: %s"

[schinagl]

This one could be finished .... ? Take over translations and clean up JAPAN? What do you think?

[schinagl]

It could be finished even without Japan cleanup, but bringing it to an end would be nice.

[schinagl]

Added #393 since this seems to have got stuck

[malxau]

I don't think this is stuck exactly. I'm trying to apply the same standard to my own changes as other people's, meaning avoiding committing things if other people don't approve them. Here it doesn't look like there's entrenched disagreement - we all seem to agree that the current JAPANBEGIN stuff can't be right - but haven't dug in to see why it's there. If we can't agree, then I'm fine with #393, but it seems like we should give agreement a chance.

I set up a VM to try this out. It's harder than it should be, because Winfile will load ja-JP resources based on the user's display language, but the bJapan flag is driven by the system locale, and those aren't the same thing. So my initial attempt showed nothing happening. After setting the system locale, the point of these becomes clear - Winfile uses a single MOVECOPYDIALOG resource and SuperDlgProc in code for Copy, Move, Rename, Hardlink, and Symlink. The English version of this dialog uses "From:" and "To:" as labels, which are operation-agnostic and seem correct. In Japanese, these labels get tailored to the operation:

In hindsight, this doesn't seem strange or unreasonable, and it's a little culture blind to assume that these labels must be generic when translated to other languages.

As of now, I think the "right" thing to do here is to fork MOVECOPYDIALOG into five dialogs. The code was clearly trying to optimize for efficiency in an era of floppy disk distribution and 640Kb of RAM, where using one dialog for move, copy and rename was a compactness win. Today the issue is more about maintenance than size, although what's happening with bJapan doesn't seem like a highly maintainable approach. If these had been five dialogs, I doubt we'd need to debug what it's doing in order to reason about it; and if the user changes their display language or winfile display language, it would do the right thing.

Unrelated to the issue with resources, but writing for the record after digging into it, bJAPAN also controls font sizes (on the assumption that Japanese file names will have fewer characters but need to be rendered larger to be legible.) That part makes sense to retain, although the check for Japanese seems a bit off, and presumably zh-CN wants the same behavior. The other thing it's doing is using WideCharToMultiByte to see if the multi-byte representation of a file name fits in the 8.3 8-bit file name constraint. This seems comically outdated today, because the whole point of NT/NTFS is that any system can have Unicode file names (we'd need to do this check regardless of language) but shortly after the code was added, FAT got long file name support with Unicode characters, rendering the whole point of the check obsolete. Technically somebody could set the Win31FileSystem registry key, but if they do, this will be the last of their worries.

Also unrelated, I never understood the "Other:" text at the bottom of MOVECOPYDIALOG. It looks like it's trying to show the current directory of drives other than the current one. Since my VM only has one C: drive, the result is non-localized Other: without anything following. Although I don't think it belongs in this PR, it probably makes sense to hide "Other:" if there are no other drives, and it probably makes sense to use a localized string for this.

[clzls]

Thx @malxau for deep research. (I'm a Japanese learner, so refer to (potential) native Japanese speakers, @Takym or so, for further discussions.)

Maybe we could just break these stuff, JAPANBEGIN or bJapan, into several parts and remove them gradually, keeping inconsistencies about JAPANBEGIN as-is before we're done.

I dig into res_ja-JP.rc and realize that the tricky part is we Asians treat IDS_KK_COPYFROMSTR and IDS_KK_RENAMEFROMSTR (along with their counterparts *TOSTR) differently: IDS_KK_COPYFROMSTR means Source to copy from, IDS_KK_RENAMEFROMSTR means Original name/path (of the item). A simple non-operation-agnostic string (in zh-CN 从(From ...) or in ja-JP 元(Source)) could be confusing but not too hard to realize what it is. In my opinion, forking MOVECOPYDIALOG into five dialogs is preferable. If that is not possible, simply using non-operation-agnostic strings seem to be acceptable.

About font size: I guess it's because Kanji / Hanzi was hard to read when on 640x480 or smaller screens. It's probably not the case nowadays, we could assume that users use large screens for reading Kanji now? (I remember that we could adjust "font size for dialog text" as part of "desktop appearance / theme" settings in Win 98 and Win XP, but I couldn't find it on Win 10 now...) Edit: observed that it try to shrink the font size to fit in the dialog, making Kanji harder to read. We may just enlarge the dialog to fit... (Top: ja-JP; Bottom: zh-CN; both contains Kanji / Hanzi)

About Other:: It is not localized for zh-CN too. I'm using Winfile 10.2.0.0 release version on my computer, which do have several drives. (A single word "Other" is confusing even in English, right? I didn't know what it means before this post...)

Unrelated: IDS_WRNNOSHIFTJIS warns about a font that do not support Shift-JIS charset is chosen. Where is this warning issued? Is it functioning now? Is it extendable for other languages to check whether a font covers their ANSI charset?

Also unrelated: I guess 'KK' in 'IDS_KK' was a shorthand of Katakana, used when OS / fonts could not handle Kanji / Hanzi correctly. But I have seen no evidence. (Kanji was introduced in these KK's as early as in win31rc)

[schinagl]

I don't think this is stuck exactly. I'm trying to apply the same standard to my own changes as other people's, meaning

After 3 month I assumed it was stuck ....

Winfile uses a single MOVECOPYDIALOG resource and SuperDlgProc in code for Copy, Move, Rename, Hardlink, and Symlink. The English version of this dialog uses "From:" and "To:" as labels, which are operation-agnostic and seem correct. In Japanese, these labels get tailored to the operation:

Good to see you found it out yourself even if I explained it some time ago

As of now, I think the "right" thing to do here is to fork MOVECOPYDIALOG into five dialogs. The code was clearly trying to

Having 5 dialogs is the worst choice. With this issue/discussion you are trying to solve a non-existant problem.

• Nobody complained
• There is no malfunction
• When done no one will have a benfit
• You are adressing a cosmetic thing
• The only ROI is, that verifyresources will have one complaint less

[malxau]

Having 5 dialogs is the worst choice.

What is your proposal? The reason this appeared stuck is because the earlier comment "clean up JAPAN", seemed like it was requesting a change without explicitly describing that change, so my post was a sincere attempt to explore what a cleanup might look like.

[schinagl]

As mentioned here:

• Either keep it as it is
• Remove it from all languages except Japanese

In both cases a comment in the source why it is as it is will help future developers to understand this.

Please read the hyperlinked comments above

[malxau]

Okay, this explains a few things: I do not see the comment that is being linked to. That link takes me to the res_zh-CN.rc file, which contains the comment from clzls, but I do not see a comment from you. This is starting to sound like a big miscommunication/misunderstanding.

We could remove JAPANBEGIN/JAPANEND since those are always true in the current code.

That said, I think there is a lot of value in having verifyresources complete without reporting issues, one way or another. This PR showed that it is effective at finding real issues.

[schinagl]

See the screenshot from my comment a few month ago... You should be able to read all comments of the forum. Try and scroll from the beginning. Anyhow: Below is a screenshot

[clzls]

Okay, this explains a few things: I do not see the comment that is being linked to. That link takes me to the res_zh-CN.rc file, which contains the comment from clzls, but I do not see a comment from you. This is starting to sound like a big miscommunication/misunderstanding.

I couldn't see the comment at my side, either, and nothing show up in this PR's review log. (The "Pending" badge seems to be suspicious?)

IDD_KK_TEXTTO and IDD_KK_TEXTFROM never "covered nicely in any language", at least in Chinese. It just "covered the MINIMUM need", as I mentioned before:

I dig into res_ja-JP.rc and realize that the tricky part is we Asians treat IDS_KK_COPYFROMSTR and IDS_KK_RENAMEFROMSTR (along with their counterparts *TOSTR) differently: IDS_KK_COPYFROMSTR means Source to copy from, IDS_KK_RENAMEFROMSTR means Original name/path (of the item). A simple non-operation-agnostic string (in zh-CN 从(From ...) or in ja-JP 元(Source)) could be confusing but not too hard to realize what it is.

I personally would prefer the verb in those tips for Chinese to improve readability, but since IDD_KK_TEXTTO's "covered the minimum need", I'd prefer to reserve my opinion for now. Any of:

• Using IDS_KK_*FROMSTR for other languages (improving readability for some languages),
• Maintaining the status quo,
• Removing it from all languages but ja-JP, or
• Removing it from all languages including ja-JP (if they agree, since non-operation-agnostic strings are confusing but readable, like zh-CN)

is acceptable for me.

[schinagl]

These are code reviews @clzls : you commented on one code review.

So it seems we have a major malfunction of the GitHub website here if you can't see all comments for code-reviews.

Then I assume you missed many other comments on code reviews too.

Would it make sense to file a bug report to GitHub?

[malxau]

The Github process for code reviews allows a reviewer to make many comments, each of which are pending, then submit all comments as part of completing a review. This is to allow a review to make comments on a large review where some earlier comments might be answered or modified as the reviewer reads more of the code. The final step of completing the review is required to publish these comments. This is documented in https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request , after step 9.

With this review itself though, I think:

• There's a strong benefit to having verifyresources complete without complaint; making this happen found real issues. If it has overactive complaints, we can't expect people to dig through to distinguish signal from noise.

• The situation where bJapan is out of sync with the language resources used is strange and unexpected, but also problematic. In the case I observed, Japanese dialogs were not updated with IDS_KK_ text. There may be a worse case, which I have not observed, where non-Japanese dialogs are updated with IDS_KK_ text; in the current code, this would update the dialog to have empty strings (LoadString adds a NULL terminator when it fails in this case.)

With that, as of now, my preferences in order are:

1. Use IDS_KK_ for all languages. For some languages this is overhead and contains identical strings, but it gives us the semantics to improve zh-CN or other languages in future. It also means we can remove the bJapan handling here, which eliminates any potential inconsistencies and simplifies the code; note that the assumption is that string resources will be loaded from the same language as dialog resources, so the two will remain in sync. bJapan would still be retained for the font size and file name checks, or at least, changes to those don't seem to belong in this PR.
2. Use five dialogs. I appreciate schinagl's concern here, and all other things being equal, maintaining five things is more effort than maintaining one thing. However, maintaining a large volume of simple things can also be easier than a small volume of complex things, and the way one language is treated differently here seems to pose maintenance challenges. Doing this makes it possible for verifyresources to improve - for example, it could identify the font of a dialog and ensure text fits within control dimensions. That is only possible if dialog resources are kept simple without external modifications.
3. Remove the IDS_KK_ logic completely, and use generic strings for all languages. Initially I assumed this to be unacceptable, and as a non-Japanese speaker am not qualified to assess how bad it would be, but clzls comment above suggests it may be acceptable.

@clzls Going back to the IDS_WRNNOSHIFTJIS question, I'm not knowledgeable in this area. From what I can see in the debugger, this isn't functional anymore. The logic occurs when the user selects a new font from the dialog. On a Japanese system, this always falls back to a SHIFTJIS_CHARSET capable font, even within the dialog itself (the preview pane will use a fallback font rather than the selected font.) For example, it's possible to select Fixedsys (a pure bitmap non Unicode font), Winfile will see the selection as Fixedsys indicating it supports SHIFTJIS_CHARSET, and when the font is instantiated, a totally different font will be used instead, and the warning is not displayed.

[schinagl]

1. Use `IDS_KK_` for all languages.  For some languages this is overhead and contains identical strings, but it gives us the semantics to improve `zh-CN` or other languages in future.  It also means we can remove the `bJapan` handling here, which eliminates any potential inconsistencies and simplifies the code; note that the assumption is that string resources will be loaded from the same language as dialog resources, so the two will remain in sync.  `bJapan` would still be retained for the font size and file name checks, or at least, changes to those don't seem to belong in this PR.

You can't use IDSKK for all languages and remove 'if (bJapan)', because dialogs will then show strings for IDDKK in all languages. Only Japanese has the need to show this, because they need the verb in a different place.

So the whole IDSKK and IDDKK stuff is a means to somehow 'overload' the default dialogs for languages with different semantics as Japanese.

So 1) flaws

So let me rephrase 0)

• Keep the IDSKK, IDDKK stuff and bJapan as it is
• Save the current output of verifyresources as kind of baseline. Any further run of verifyresources is compared to this baseline and according action is taken
• Make a comment in the source how this works

[schinagl]

Until you have decided how to proceed with this PR, please merge #393

[malxau]

I pushed an update that implements option 1.

This has the drawback of duplicating strings in languages that don't require different strings. It has the advantage that all languages behave the same way, so zh-CN which wants to behave like ja-JP now can. Verifyresources will no longer detect inconsistencies, since languages are consistent.

This preserves the previous code behavior where the dialog text refers to "move."

I really think keeping languages behaving consistently will make it easier to make changes in future.