Remove {Locked} from localization machinery and localize the version command. #463

BillyONeal · 2022-03-29T00:13:19Z

No description provided.

…command.

BillyONeal · 2022-03-29T00:14:33Z

src/vcpkg.cpp

@@ -336,16 +320,23 @@ int main(const int argc, const char* const* const argv)
    LockGuardPtr<Metrics>(g_metrics)->track_property("error", exc_msg);

    fflush(stdout);
-    msg::println(msgVcpkgHasCrashed, msg::email = Commands::Contact::email());
+    msg::println(LocalizedString::from_raw("vcpkg.exe has crashed."));


This PR preserves the previous behavior, but I think this should be localized?

Side note, everyone I know from Mexico Spanglishes "has crashed" as "crasheo".

It looks to me like this was actually localized before.

DECLARE_AND_REGISTER_MESSAGE(VcpkgHasCrashed, (msg::email), "", R"(vcpkg.exe has crashed...)");

Just this section was locked previously.

DECLARE_AND_REGISTER_MESSAGE(VcpkgHasCrashedDataBlob, (msg::version, msg::error), "{Locked}", R"( Version={version} EXCEPTION='{error}' CMD=)");

src/vcpkg/install.cpp

include/vcpkg/base/graphs.h

ras0219-msft · 2022-03-29T02:31:57Z

src/vcpkg/install.cpp

+                    msg.append(LocalizedString()
+                                   .append_indent()
+                                   .append_raw(fmt::format("target_link_libraries(main PRIVATE {})",
+                                                           Strings::join(" ", targets)))
                                   .appendnl()
                                   .appendnl()
                                   .extract_data());


Suggested change

msg.append(LocalizedString()

.append_indent()

.append_raw(fmt::format("target_link_libraries(main PRIVATE {})",

Strings::join(" ", targets)))

.appendnl()

.appendnl()

.extract_data());

msg.append(fmt::format("target_link_libraries(main PRIVATE {})\n\n", Strings::join(" ", targets));

You lost the indent here?

src/vcpkg/install.cpp

ras0219-msft

LGTM with Victor's and my comments.

strega-nil-ms · 2022-03-29T17:18:53Z

src/vcpkg.cpp

    fflush(stdout);
    for (int x = 0; x < argc; ++x)
    {
 #if defined(_WIN32)
-        msg::println(msgVcpkgHasCrashedArgument, msg::value = Strings::to_utf8(argv[x]));
+        msg::println(LocalizedString::from_raw(Strings::to_utf8(argv[x]) + "|"));


Printing for each line here kinda sucks; I'd rather do something like:

LocalizedString arguments; for (...) { arguments.append_raw(Strings::to_utf8(argv[x])).append_raw("|").append_nl(); }

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

ras0219-msft · 2022-03-30T01:10:50Z

src/vcpkg.cpp

+    LocalizedString data_blob;
+    data_blob.append_raw("Version=").append_raw(Commands::Version::version).appendnl();
+    data_blob.append_raw("EXCEPTION=").append_raw(exc_msg).appendnl();
+    data_blob.append_raw("CMD=").appendnl();
    for (int x = 0; x < argc; ++x)
    {
 #if defined(_WIN32)
-        msg::println(LocalizedString::from_raw(Strings::to_utf8(argv[x]) + "|"));
+        data_blob.append_raw(Strings::to_utf8(argv[x])).append_raw("|").appendnl();
 #else
-        msg::println(LocalizedString::from_raw(std::string(argv[x]) + "|"));
+        data_blob.append_raw(argv[x]).append_raw("|").appendnl();
 #endif
    }
+
+    msg::print(data_blob);


Suggested change

LocalizedString data_blob;

data_blob.append_raw("Version=").append_raw(Commands::Version::version).appendnl();

data_blob.append_raw("EXCEPTION=").append_raw(exc_msg).appendnl();

data_blob.append_raw("CMD=").appendnl();

for (int x = 0; x < argc; ++x)

{

#if defined(_WIN32)

msg::println(LocalizedString::from_raw(Strings::to_utf8(argv[x]) + "|"));

data_blob.append_raw(Strings::to_utf8(argv[x])).append_raw("|").appendnl();

#else

msg::println(LocalizedString::from_raw(std::string(argv[x]) + "|"));

data_blob.append_raw(argv[x]).append_raw("|").appendnl();

#endif

}

msg::print(data_blob);

std::string msg = Strings::concat("Version=", Commands::Version::version, "\n", "EXCEPTION=", exc_msg, "\n", "CMD=\n");

for (int x = 0; x < argc; ++x)

{

#if defined(_WIN32)

Strings::append(msg, Strings::to_utf8(argv[x]), "|\n");

#else

Strings::append(msg, argv[x], "|\n");

#endif

}

print2(msg);

This goes backwards: the intent is for that everything that has been audited for localization goes through msg::print so that a 'find all references' on print2 finds everything yet to be fixed.

strega-nil-ms · 2022-03-30T17:40:43Z

src/vcpkg/commands.version.cpp

+        VersionCommandHeader,
+        (msg::version),
+        "",
+        "Vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.");


Suggested change

"Vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.");

"vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.");

nit, but it's spelled vcpkg; close as wontfix if you don't want to change behaviour

BillyONeal · 2022-03-31T00:57:02Z

/azp run

azure-pipelines · 2022-03-31T00:57:12Z

Azure Pipelines successfully started running 1 pipeline(s).

Remove {Locked} from localization machinery and localize the version …

9ecf97f

…command.

BillyONeal mentioned this pull request Mar 29, 2022

[docs] add localization doc #462

Merged

BillyONeal commented Mar 29, 2022

View reviewed changes

Drop static from g_shutdown_handler.

130adf2

ras0219-msft reviewed Mar 29, 2022

View reviewed changes

src/vcpkg/install.cpp Outdated Show resolved Hide resolved

ras0219-msft reviewed Mar 29, 2022

View reviewed changes

include/vcpkg/base/graphs.h Outdated Show resolved Hide resolved

ras0219-msft reviewed Mar 29, 2022

View reviewed changes

src/vcpkg/install.cpp Show resolved Hide resolved

ras0219-msft approved these changes Mar 29, 2022

View reviewed changes

strega-nil-ms suggested changes Mar 29, 2022

View reviewed changes

strega-nil-ms self-requested a review March 29, 2022 17:20

BillyONeal and others added 4 commits March 29, 2022 16:23

Apply suggestions from Robert

cd65f8e

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

Merge remote-tracking branch 'origin/main' into remove-locked

8b7c8fd

Code review feedback.

32d85d0

clang-format + messages

f50776d

BillyONeal requested review from ras0219-msft and vicroms March 30, 2022 00:35

ras0219-msft approved these changes Mar 30, 2022

View reviewed changes

strega-nil-ms reviewed Mar 30, 2022

View reviewed changes

strega-nil-ms approved these changes Mar 30, 2022

View reviewed changes

BillyONeal and others added 3 commits March 30, 2022 13:29

Merge remote-tracking branch 'origin/main' into remove-locked

b2cef8c

Lowercase vcpkg and drive by remove loop in messages.h.

f0f8000

std::size_t -> size_t

5f0dba6

vicroms approved these changes Mar 30, 2022

View reviewed changes

strega-nil-ms merged commit 904440f into microsoft:main Mar 31, 2022

BillyONeal deleted the remove-locked branch April 1, 2022 02:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove {Locked} from localization machinery and localize the version command. #463

Remove {Locked} from localization machinery and localize the version command. #463

BillyONeal commented Mar 29, 2022

BillyONeal Mar 29, 2022

vicroms Mar 29, 2022

vicroms Mar 29, 2022 •

edited

Loading

ras0219-msft Mar 29, 2022

BillyONeal Mar 29, 2022

ras0219-msft left a comment

strega-nil-ms Mar 29, 2022

ras0219-msft Mar 30, 2022 •

edited

Loading

BillyONeal Mar 30, 2022

strega-nil-ms Mar 30, 2022

BillyONeal commented Mar 31, 2022

azure-pipelines bot commented Mar 31, 2022

	"Vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.");
	"vcpkg package management program version {version}\n\nSee LICENSE.txt for license information.");

Remove {Locked} from localization machinery and localize the version command. #463

Remove {Locked} from localization machinery and localize the version command. #463

Conversation

BillyONeal commented Mar 29, 2022

BillyONeal Mar 29, 2022

Choose a reason for hiding this comment

vicroms Mar 29, 2022

Choose a reason for hiding this comment

vicroms Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

ras0219-msft Mar 29, 2022

Choose a reason for hiding this comment

BillyONeal Mar 29, 2022

Choose a reason for hiding this comment

ras0219-msft left a comment

Choose a reason for hiding this comment

strega-nil-ms Mar 29, 2022

Choose a reason for hiding this comment

ras0219-msft Mar 30, 2022 • edited Loading

Choose a reason for hiding this comment

BillyONeal Mar 30, 2022

Choose a reason for hiding this comment

strega-nil-ms Mar 30, 2022

Choose a reason for hiding this comment

BillyONeal commented Mar 31, 2022

azure-pipelines bot commented Mar 31, 2022

vicroms Mar 29, 2022 •

edited

Loading

ras0219-msft Mar 30, 2022 •

edited

Loading