Localize option strings#35
Conversation
|
@benmcmorran, |
| public const string PageGeneralName = "General"; | ||
| public const string PageParallelizationName = CategoryParallelizationName; | ||
| public const string PageGoogleTestName = "Google Test"; | ||
| public static string CategoryTestExecutionName = Resources.CategoryTestExecutionName; |
There was a problem hiding this comment.
All the strings that were const should become static **readonly**.
| <comment>{0} represents the path of a test executable</comment> | ||
| </data> | ||
| <data name="DescriptionOfSolutionDirPlaceHolder" xml:space="preserve"> | ||
| <value>{0} - directory of the solution (only available inside VS)</value> |
There was a problem hiding this comment.
Please say "Visual Studio".
| TestDirPlaceholder + " - path of a directory which can be used by the tests\n" + | ||
| ThreadIdPlaceholder + " - id of thread executing the current tests\n" + | ||
| DescriptionOfSolutionDirPlaceHolder; | ||
| // Set in constructor because it depends on other strings |
There was a problem hiding this comment.
Is this to avoid initialization order issues? Aren't fields initialized top-to-bottom anyway?
There was a problem hiding this comment.
You're right. I think this is left over from back before I marked all the fields static.
| </data> | ||
| <data name="DescriptionOfPlaceholdersForBatches" xml:space="preserve"> | ||
| <value>{0} - path of a directory which can be used by the tests | ||
| {1} - id of thread executing the current tests</value> |
| public const string OptionTestDiscoveryRegexDescription = | ||
| "If non-empty, this regex will be used to discover the Google Test executables containing your tests.\nDefault regex: " | ||
| + TestFinderRegex; | ||
| public static string OptionTestDiscoveryRegexDescription = Resources.OptionTestDiscoveryRegexDescription + TestFinderRegex; |
There was a problem hiding this comment.
Shouldn't TestFinderRegex rather be a fill-in for string.Format?
| </data> | ||
| <data name="OptionParseSymbolInformationDescription" xml:space="preserve"> | ||
| <value>Parse debug symbol information for test executables to obtain source location information and traits (defined via the macros in GTA_Traits.h). | ||
| If this is set to false step 2 of traits discovery will be left out and only traits regexes will be effective.</value> |
| <value>Kill processes on cancel</value> | ||
| </data> | ||
| <data name="OptionKillProcessesOnCancelDescription" xml:space="preserve"> | ||
| <value>If true, running test executables are actively killed if the test execution is canceled. Note that killing a test process might have all kinds of side effects; in particular, Google Test will not be able to perform any shutdown tasks.</value> |
There was a problem hiding this comment.
Please use "terminate" verbiage instead.
| [Category(SettingsWrapper.CategoryTestExecutionName)] | ||
| [DisplayName(SettingsWrapper.OptionTestDiscoveryRegex)] | ||
| [Description(SettingsWrapper.OptionTestDiscoveryRegexDescription)] | ||
| [LocalizedCategory("CategoryTestExecutionName")] |
There was a problem hiding this comment.
It appears to me that this change is not strictly necessary, as the code as before would have been reading localized strings from SettingsWrapper. What is the benefit of this indirection?
There was a problem hiding this comment.
Attribute parameters must be compile-time constant, but the localized strings are not known until runtime. The indirection allows us to defer actual localized string lookup until runtime, while still having a constant parameter in the attribute.
| <data name="400" type="System.Resources.ResXFileRef, System.Windows.Forms"> | ||
| <value>..\..\Resources\Icons\Icon.ico;System.Drawing.Icon, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a</value> | ||
| </data> | ||
| <data name="501" xml:space="preserve"> |
There was a problem hiding this comment.
Could you explain to me how this works?
There was a problem hiding this comment.
The name (number) is used in GoogleTestExtensionOptionsPage.cs:26 to specify localized lookups for the category and page names in the options window:
[ProvideOptionPage(typeof(GeneralOptionsDialogPage), OptionsCategoryName, "General", 110, 501, true)]
Parameters 2 and 3 specify non-localized canonical names for the category and page names, while parameters 3 and 4 specify resource ids for the corresponding localized versions. ProvideOptionPage only allows shorts in these positions, so we are forced to use numbers.
There was a problem hiding this comment.
Thanks for the explanation.
| </data> | ||
| <data name="501" xml:space="preserve"> | ||
| <value>General</value> | ||
| <comment>The name of page of options</comment> |
There was a problem hiding this comment.
Similar comment is suspiciously missing from the entries below.
There was a problem hiding this comment.
Fixed in new commit.
| </data> | ||
| <data name="503" xml:space="preserve"> | ||
| <value>Google Test</value> | ||
| <comment>The name of page of options</comment> |
There was a problem hiding this comment.
Should we add a comment (if it makes sense) that this is a name of the library and should probably not be localized?
3 participants
@Microsoft/vcls please review