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

The localization of Chinese has been completed #506

Merged
merged 46 commits into from Apr 28, 2018

Conversation

Projects
None yet
3 participants
@maikebing
Contributor

maikebing commented Dec 19, 2017

  • Extract all strings and WPF strings that need to be translated And the use of TomEnglert.ResXManager for translation work, of which Chinese has been translated.

maikebing added some commits Dec 19, 2017

Extract all strings and WPF strings that need to be translated And th…
…e use of TomEnglert.ResXManager for translation work, of which Chinese has been translated.
@maikebing

This comment has been minimized.

Contributor

maikebing commented Dec 27, 2017

@codecadwallader do you have time merge it ?

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Dec 28, 2017

I wasn't sure if you were ready for review, I will look over it.

<EmbeddedResource Include="source.extension.resx">
<AutoGen>True</AutoGen>
<DesignTime>True</DesignTime>
<DependentUpon>source.extension.vsixmanifest</DependentUpon>
<MergeWithCTO>true</MergeWithCTO>
<ManifestResourceName>VSPackage</ManifestResourceName>
</EmbeddedResource>
<EmbeddedResource Include="source.extension.zh-Hans.resx">

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

I wasn't aware you could have a resources translation of the manifest file. How does this work?

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

Visual Studio compiles automatic local language using different manifest files

@@ -288,13 +288,15 @@
If you do not want an image next to your command, remove the Icon node or set it to <Icon guid="guidOfficeIcon" id="msotcidNoIcon" /> -->
<Button guid="GuidCodeMaidCommandAbout" id="CmdIDCodeMaidAbout" priority="0x0100" type="Button">
<Icon guid="GuidImageInfo" id="IconInfo" />
<CommandFlag>TextChanges</CommandFlag>

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

Making the text dynamic can hurt performance, since Visual Studio will re-evaluate it constantly. Does this have to be done, or is there another way to point the VSCT definitions directly to resource keys?

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

Except for the dynamic text, I didn't find a better way to do it.There seems to be an article that says VSCT can have multiple languages. But I don't know how to do it so far.

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

I think looking into this further will be necessary.. in the past the performance hit was debilitating as our extension as well as other ones would cause each other to infinitely regenerate the text.

This comment has been minimized.

@maikebing

maikebing Jan 3, 2018

Contributor

I tried the method described in MSDN, but I didn't succeed and I didn't know what the problem was.
url: https://msdn.microsoft.com/en-us/library/ee943168.aspx

This comment has been minimized.

@maikebing

maikebing Jan 3, 2018

Contributor

The MSDN approach has been successful !
3642dec

@@ -48,12 +48,12 @@ protected override void OnExecute()
if (!CodeCleanupAvailabilityLogic.IsCleanupEnvironmentAvailable())
{
MessageBox.Show(@"Cleanup cannot run while debugging.",
@"CodeMaid: Cleanup All Code",
MessageBox.Show(StringResourceKey.CleanupAllCodeCommand_OnExecute_CleanupCannotRunWhileDebugging,

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

Some of these key names are really long, compared to other ones which are much shorter. I'd prefer the shorter versions - I don't think the whole path of where every key came from is necessary.

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

I used tom-englert/ResXResourceManager to achieve string extraction, and I used the variable name that it automatically generated.In order to complete the extraction of string statements as soon as possible, I have no refactoring.

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

I would like to have these simplified before accepting the pull request.. otherwise the code will be pretty difficult to read.

This comment has been minimized.

@maikebing

maikebing Jan 3, 2018

Contributor

Code has been refactored e279ed8

@@ -27,7 +27,7 @@ internal CommentFormatCommand(CodeMaidPackage package)
: base(package,
new CommandID(PackageGuids.GuidCodeMaidCommandCommentFormat, PackageIds.CmdIDCodeMaidCommentFormat))
{
_undoTransactionHelper = new UndoTransactionHelper(package, "CodeMaid Format Comment");
_undoTransactionHelper = new UndoTransactionHelper(package, StringResourceKey.CommentFormatCommand_CommentFormatCommand_CodeMaidFormatComment);

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

Instead of StringResourceKey, you could access the Resources directly like you do in some cases below.

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

StringResourceKey is the name of the string resource class that is used by default when tom-englert/ResXResourceManager is automatically generated. It's not perfect, but it's the best localization translation tool I've ever met.

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

Sometimes StringResourceKey is used.. sometimes Resources is used. I think to avoid confusion Resources should always be used where possible.

This comment has been minimized.

@maikebing

maikebing Jan 2, 2018

Contributor

Using resources requires referencing namespace SteveCadwallader.CodeMaid.Properties, but using Stringresourcekey does not require Stevecadwallader.codemaid, Because the root namespace is it. It would be awkward to add namespaces to all the required code SteveCadwallader.CodeMaid.Properties. However, it is easier to refer to SteveCadwallader.CodeMaid.Properties in WPF.

<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="BuildProgressToolWindow_BuildProgress" xml:space="preserve">
<value>生成进度</value>

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

How were all of these translations done? Was it an automated tool, did you do them manually? Since I don't speak Chinese I am concerned if any of them may be offensive, confusing or misleading.

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

The first is through the use of Tom-englert/resxresourcemanager Microsoft translation, and then manual proofreading., It conforms to the Chinese language grammar.

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

Ok, thanks.

namespace SteveCadwallader.CodeMaid
{
internal class StringResourceKey : Properties.Resources

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

I think I know what's going on here, but can you explain this file to me to make sure?

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

Stringresourcekey is the name of the string resource class that is used by default when Tom-englert/resxresourcemanager is automatically generated. It's not perfect, but it's the best localized translation tool I've ever seen. Use of this file is still to cater to the tool to make the translation faster completion.

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

Ok, but can you explain what this file is doing to me?

This comment has been minimized.

@maikebing

maikebing Jan 3, 2018

Contributor

Using resources requires referencing namespace SteveCadwallader.CodeMaid.Properties, but using Stringresourcekey does Not require Stevecadwallader.codemaid, because the root namespace is it. It would be awkward to add namespaces to all required code SteveCadwallader.CodeMaid.Properties.

This comment has been minimized.

@codecadwallader

codecadwallader Mar 11, 2018

Owner

It looks like this class isn't doing anything at all anymore, so we could delete it and have everything point consistently to Resources. I know it means adding a using statement to do those C# files, but I do not think that is a bad thing and I think it's simpler than having two seeming versions of the resources.

@@ -176,7 +176,7 @@ private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEven
dynamic currentItem = e.UserState;
CountProgress = currentCount;
CurrentFileName = currentItem.Name;
CurrentFileName = string.Format(StringResourceKey.Cleaning0, currentItem.Name);

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

This isn't the current file name anymore, but an output string. I think this was done to avoid using the StringFormat in XAML. If possible it would be preferable to keep the StringFormat in XAML, since this class is a ViewModel.

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

I tried the XAML stringformat, but it doesn't seem to work.

This comment has been minimized.

@codecadwallader

codecadwallader Mar 11, 2018

Owner

Ok, another approach would be to split the label into two components then. One for the "Cleaning" which can be translated directly and one for the CurrentFileName. That would allow this ViewModel to be reverted to not having View code in it.

@@ -1,6 +1,7 @@
<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="clr-namespace:SteveCadwallader.CodeMaid.UI.Dialogs.Options.Cleaning"
xmlns:properties="clr-namespace:SteveCadwallader.CodeMaid.Properties"

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

In other files the convention is xmlns:p so it would be good to stick to that here too.

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

'properties' are the names that Tom-englert/resxresourcemanager use by default, so they are still for faster translation.

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

I understand a lot of the code was auto-generated by a tool, but we've been really diligent about trying to have as clean of a code base as possible. To accept the pull request the auto-generated code will have to be updated to match conventions of the existing code base.

This comment has been minimized.

@maikebing

maikebing Jan 3, 2018

Contributor

has been modified !
91af8fd

FileName = "CodeMaid",
DefaultExt = ".config",
Filter = "Config files (*.config)|*.config|All Files (*.*)|*.*"
Filter = StringResourceKey.OptionsViewModel_OnImportCommandExecuted_ConfigFilesConfigConfigAllFiles

This comment has been minimized.

@codecadwallader

codecadwallader Dec 28, 2017

Owner

This auto-generated name is very awkward.. perhaps ConfigFileFilter would be clearer.

This comment has been minimized.

@maikebing

maikebing Dec 28, 2017

Contributor

Yes, it's very long, I submit a refactoring code?

This comment has been minimized.

@codecadwallader

codecadwallader Jan 2, 2018

Owner

That would be great, thanks.

This comment has been minimized.

@maikebing
@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 7, 2018

Is there anything else you need to change? @codecadwallader

@codecadwallader

It is looking really good and I think we are getting close. I do have a couple continuing requests. Please let me know if you have any questions and thanks for your patience!

<Compile Include="CodeMaid.cs">
<AutoGen>True</AutoGen>

This comment has been minimized.

@codecadwallader

codecadwallader Mar 11, 2018

Owner

Since this is an auto-generated file, I think we need this flag?

@@ -46,6 +46,10 @@ protected BaseCommand(CodeMaidPackage package, CommandID id)
private static void BaseCommand_BeforeQueryStatus(object sender, EventArgs e)
{
BaseCommand command = sender as BaseCommand;
//if (string.IsNullOrEmpty(command.Text))

This comment has been minimized.

@codecadwallader

codecadwallader Mar 11, 2018

Owner

Please remove commented out code.

namespace SteveCadwallader.CodeMaid
{
internal class StringResourceKey : Properties.Resources

This comment has been minimized.

@codecadwallader

codecadwallader Mar 11, 2018

Owner

It looks like this class isn't doing anything at all anymore, so we could delete it and have everything point consistently to Resources. I know it means adding a using statement to do those C# files, but I do not think that is a bad thing and I think it's simpler than having two seeming versions of the resources.

@@ -176,7 +176,7 @@ private void backgroundWorker_ProgressChanged(object sender, ProgressChangedEven
dynamic currentItem = e.UserState;
CountProgress = currentCount;
CurrentFileName = currentItem.Name;
CurrentFileName = string.Format(StringResourceKey.Cleaning0, currentItem.Name);

This comment has been minimized.

@codecadwallader

codecadwallader Mar 11, 2018

Owner

Ok, another approach would be to split the label into two components then. One for the "Cleaning" which can be translated directly and one for the CurrentFileName. That would allow this ViewModel to be reverted to not having View code in it.

Modify for PR #506 ,Removed commented out code, Removed "StringResour…
…ceKey",Modify CleanupProgressViewModel
@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 14, 2018

I have finished. Take the time to review the code!

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Mar 17, 2018

Thank you for the code updates. The code is looking good and I have no further comments. 👍

I pulled down the code and tried to run it locally, but when I startup an experimental instance of Visual Studio the CodeMaid menu is not present at all. I suspect the changes to the .vsct or source.extension files have caused an issue. Can you please look into it?

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 17, 2018

ResetEnvironment.zip

try this cmd , it reset you VS !

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Mar 17, 2018

Yeah I tried the usual "Reset the Visual Studio 2017 Experimental Instance" batch file that comes with the SDK but it did not fix the issue.

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 18, 2018

My VS2017 version is 15.6.2. The code I tested under the master branch doesn't seem to be working properly, and I'll find out why.

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 22, 2018

I tested it and looked normal together.
I uninstalled the codemaid and removed all appdatalocalmicrosoftvisualstudio15.0_f203e778exp. The associated registry key is also deleted.
When I am debugging, I can enter the debugging, also can display the Codemaid menu!

here are also files in the directory
C:\Users\MaiKeBing\AppData\Local\Microsoft\VisualStudio\15.0_f203e778Exp\Extensions\Steve Cadwallader\CodeMaid\10.4

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 22, 2018

But if I use the parameter/rootsuffix EXP1 then the Codemaid menu does not show

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 22, 2018

Change the parameters anyway, VS2017 will put codemaid here.
C:\Users\MaiKeBing\AppData\Local\Microsoft\VisualStudio\15.0_f203e778Exp

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 22, 2018

If you use other parameters, please try “/rootsuffix Exp”

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Mar 24, 2018

Yep /rootsuffix Exp is baked into the repository: https://github.com/codecadwallader/codemaid/blob/master/CodeMaid/CodeMaid.csproj.user#L6

I'm not sure I follow your comments about what you tried and what happened?

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Mar 24, 2018

As a heads up - I just merged another pull request that allows users to turn on/off different features. It looks like your PR will still be able to merge in cleanly, but there are some new user options that will also need setup for translation. A good question is how will we perform translations into Chinese going forwards every time the UI changes? I can use Google Translate, but I know that's not a great source...

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 24, 2018

It is recommended that you use Microsoft's Bing translation tool

@maikebing

This comment has been minimized.

Contributor

maikebing commented Mar 24, 2018

I like codemaid very much, if need, I can translate at any time.

@maikebing

This comment has been minimized.

Contributor

maikebing commented Apr 9, 2018

If CodeMaid is a maid of code, "Spade" has been used in English,I think the direct translation as "A spade for digging code", So it's called 码锹( Code Spade), I think it will be a good name.!

@heku

This comment has been minimized.

Contributor

heku commented Apr 10, 2018

OK, let us use this translation at this stage. I suggest for every localization, open an issue for any feedback and discussion with a title in two languages. For example, for current Chinese localization, it can be “Chinese localization feedback/discussion/suggestion 中文本地化 反馈/讨论/建议”. how do you guys think?

heku and others added some commits Apr 10, 2018

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Apr 14, 2018

Ok, to make sure I understand correctly you're saying that each time there is a new feature with new translations needed, new issues should be created for every localization where translation is needed? That seems fine to me.

@heku

This comment has been minimized.

Contributor

heku commented Apr 14, 2018

Basically Yes, my thought was openning one issue with a title in two language (en+local lang) for every language/localization for any feedback/suggestion. But now I think this might not be a good idea as people always like to create a new issue rather than comment on an existing issue.

heku and others added some commits Apr 14, 2018

Merge pull request #4 from heku/localization
Correct a few typos
@maikebing

This comment has been minimized.

Contributor

maikebing commented Apr 21, 2018

@heku @codecadwallader Is there any question about this?

@heku

This comment has been minimized.

Contributor

heku commented Apr 21, 2018

No, I'm fine.

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Apr 22, 2018

There is a minor conflict from another accepted PR. Beyond that, is there anything else pending the two of you have in mind before I review it again?

@heku

This comment has been minimized.

Contributor

heku commented Apr 23, 2018

@codecadwallader I don't have anything to add. @maikebing, I suggest remove the source.extension.resx as it is useless.

@maikebing

This comment has been minimized.

Contributor

maikebing commented Apr 23, 2018

source.extension.resx is automatically generated each time !

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Apr 28, 2018

Ok great! Thanks both (especially @maikebing ) for your hard work on this. I will merge it in now and we can review/test from the mainline. :)

@codecadwallader codecadwallader merged commit ee6363d into codecadwallader:master Apr 28, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@maikebing maikebing deleted the maikebing:localization branch Apr 28, 2018

@codecadwallader

This comment has been minimized.

Owner

codecadwallader commented Apr 28, 2018

@heku it looks like some of your .csproj cleanups may have gotten merged out when this got merged into master. It looks like @maikebing did do a merge down from master into this PR branch so I'm not exactly sure why that happened, but perhaps it was too many changes in the same place for Git to resolve.

@heku

This comment has been minimized.

Contributor

heku commented May 2, 2018

@codecadwallader I'll have it a check on your latest code, sorry for the late response as I'm quite busy these days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment