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

[Localization] Sculptor #718

Merged
merged 6 commits into from
Oct 6, 2022

Conversation

JavierMatosD
Copy link
Contributor

Localizes messages in the following files:

  • input.cpp
  • help.cpp

@JavierMatosD JavierMatosD marked this pull request as ready for review September 28, 2022 17:13
"every dependency in the graph. For example, if no other constraints are specified (directly or "
"transitively), then the version will resolve to the baseline of the top level manifest. Baselines "
"of transitive dependencies are ignored.");
DECLARE_MESSAGE(BuiltInTriplets, (), "", "VCPKG built-in triplets:");
Copy link
Contributor

@Thomas1664 Thomas1664 Sep 29, 2022

Choose a reason for hiding this comment

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

I remember that @ AugP opened a PR in the vcpkg repo to lowercase all occurrences of vcpkg in the documentation. We might want to do this here as well? (same below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I will have to ask him :)

include/vcpkg/base/messages.h Outdated Show resolved Hide resolved
include/vcpkg/base/messages.h Outdated Show resolved Hide resolved
include/vcpkg/base/messages.h Outdated Show resolved Hide resolved
UpdateBaselineHelp,
(),
"",
"To keep your libraries up to date, the best approach is to update your baseline reference. This will "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"To keep your libraries up to date, the best approach is to update your baseline reference. This will "
"The best approach to keep your libraries up to date is to update your baseline reference. This will "

I don't have a strong opinion on this one; just a style suggestion.

"Within the \"dependencies\" field, each dependency can have a minimum constraint listed. These "
"minimum constraints will be used when transitively depending upon this library. A minimum "
"port-version can additionally be specified with a '#' suffix.");
DECLARE_MESSAGE(VersionHelp, (), "", "A dot-separated sequence of numbers (1.2.3.4)");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DECLARE_MESSAGE(VersionHelp, (), "", "A dot-separated sequence of numbers (1.2.3.4)");
DECLARE_MESSAGE(VersionHelp, (), "", "A dot-separated sequence of non-negative numbers (1.2.3.4)");

just to clarify

Copy link
Member

Choose a reason for hiding this comment

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

The goal here is for this to fit within the width of a console together, I'm not sure this clarification is the right space/clarity tradeoff.

image

@Thomas1664
Copy link
Contributor

Hi @JavierMatosD,

You have several open localization PRs. I am curious weither there will be merge conflicts if one of them gets merged since your sorting might cause interference between your PRs.

@JavierMatosD
Copy link
Contributor Author

Hi @JavierMatosD,

You have several open localization PRs. I am curious weither there will be merge conflicts if one of them gets merged since your sorting might cause interference between your PRs.

Hi @Thomas1664!

lol, yes, it causes merge conflicts. Admittedly, it is something I did not think about and is now coming back to bite me.

@Thomas1664
Copy link
Contributor

lol, yes, it causes merge conflicts. Admittedly, it is something I did not think about and is now coming back to bite me.

The way I would solve this in the future is to stop sorting and just put new messages at the end. Optionally you could alter between adding at the start vs. at the end. Then you only have to accept both changes. You can sort the messages when you're done with a Python script that splits at ; and sorts the resulting list.

# Conflicts:
#	include/vcpkg/base/messages.h
#	locales/messages.en.json
#	locales/messages.json
#	src/vcpkg/base/messages.cpp
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

LGTM with nitpick fixes and replies for Thomas1664

@@ -580,6 +580,8 @@ namespace vcpkg
"An example of env_var is \"HTTP(S)_PROXY\""
"'--' at the beginning must be preserved",
"-- Automatically setting {env_var} environment variables to \"{url}\".");
DECLARE_MESSAGE(AvailableArchitectureTriplets, (), "", "Available architecture triplets");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
DECLARE_MESSAGE(AvailableArchitectureTriplets, (), "", "Available architecture triplets");
DECLARE_MESSAGE(AvailableArchitectureTriplets, (), "", "Available architecture triplets:");

"transitively), then the version will resolve to the baseline of the top level manifest. Baselines "
"of transitive dependencies are ignored.");
tbl.header(msg::format(msgManifestConstraints));
tbl.format("builtin-baseline", msg::format(msgBuiltinBaseHelp));
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to name these so they sort together in messages.h (and to help translators realize they are related)?

tbl.text("Each version additionally has a \"port-version\" which is a nonnegative integer. When rendered as "
"text, the port version (if nonzero) is added as a suffix to the primary version text separated by a "
"hash (#). Port-versions are sorted lexographically after the primary version text, for example:");
tbl.text(msg::format(msgPortVersionHelp));
tbl.blank();
tbl.blank();
tbl.text(" 1.0.0 < 1.0.0#1 < 1.0.1 < 1.0.1#5 < 2.0.0");
Copy link
Member

Choose a reason for hiding this comment

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

I think this part should be part of msgPortVersionHelp, even if it means the blank lines are removed.

@@ -1541,6 +1588,7 @@ namespace vcpkg
"",
"Unknown setting for VCPKG_BUILD_TYPE {option}. Valid settings are '', 'debug', and 'release'.");
DECLARE_MESSAGE(UnknownTool, (), "", "vcpkg does not have a definition of this tool for this platform.");
DECLARE_MESSAGE(UnknownTopic, (msg::value), "{value} is a vcpkg topic", "unknown topic {value}");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
DECLARE_MESSAGE(UnknownTopic, (msg::value), "{value} is a vcpkg topic", "unknown topic {value}");
DECLARE_MESSAGE(UnknownTopic, (msg::value), "{value} the value a user passed to `vcpkg help` that we don't understand", "unknown topic {value}");

@@ -1681,6 +1736,20 @@ namespace vcpkg
"",
"dependency {spec} was expected to be at least version "
"{expected_version}, but is currently {actual_version}.");
DECLARE_MESSAGE(VersionDateHelp, (), "", "A date (2021-01-01.5)");
DECLARE_MESSAGE(VersionFourFlavors, (), "", "Versions in vcpkg come in four primary flavors");
Copy link
Member

Choose a reason for hiding this comment

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

Should we change this to something like "the following versions are accepted" or similar so that the message need not be retranslated if/when the set changes?

"Within the \"dependencies\" field, each dependency can have a minimum constraint listed. These "
"minimum constraints will be used when transitively depending upon this library. A minimum "
"port-version can additionally be specified with a '#' suffix.");
DECLARE_MESSAGE(VersionHelp, (), "", "A dot-separated sequence of numbers (1.2.3.4)");
Copy link
Member

Choose a reason for hiding this comment

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

The goal here is for this to fit within the width of a console together, I'm not sure this clarification is the right space/clarity tradeoff.

image

# Conflicts:
#	include/vcpkg/base/messages.h
#	locales/messages.en.json
#	locales/messages.json
#	src/vcpkg/base/messages.cpp
@JavierMatosD JavierMatosD merged commit 6386d3e into microsoft:main Oct 6, 2022
@JavierMatosD JavierMatosD deleted the Localization_Sculptor branch April 20, 2023 19:38
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.

3 participants