Localization of strings in binarycaching.cpp #606
Conversation
- Sorted Declared/Registered messages in alphabetical order.
src/vcpkg/binarycaching.cpp
Outdated
| DECLARE_AND_REGISTER_MESSAGE(HelpTableFormat, | ||
| (msg::value), | ||
| "'{value}' is vcpkg::HelpTableFormatter as a string.", | ||
| "'{value}'"); |
There was a problem hiding this comment.
This seems wrong: The old code just printed the thing, this is adding extra 's around it?
It seems like the thing itself needs to be localized rather than being washed through format like this.
There was a problem hiding this comment.
I will probably rewrite the HelpTableFormatter to require LocalizedString as input. I'll open another PR for that specifically.
src/vcpkg/binarycaching.cpp
Outdated
| print2(tbl.m_str); | ||
|
|
||
| print2("\nExtended documentation is available at ", docs::assetcaching_url, "\n"); | ||
| msg::println(msgHelpTableFormat, msg::value = tbl.m_str); |
There was a problem hiding this comment.
Here the content that actually needs to be localized is the help text blocks above rather than this part.
There was a problem hiding this comment.
Same as before
src/vcpkg/binarycaching.cpp
Outdated
| "invalid argument: binary config '{binary_source}' requires at least a prefix"); | ||
| DECLARE_AND_REGISTER_MESSAGE(InvalidArgumentRequiresSource, | ||
| (msg::binary_source), | ||
| "Extendend documentation available at '{url}'."); |
There was a problem hiding this comment.
I think it's OK to just put the URL into the localized message, IMO. But I think we are inconsistent about that at the moment...
There was a problem hiding this comment.
I am hoping to be able to reuse some of these messages in the future. Passing the URL in as an argument allows me to do that.
There was a problem hiding this comment.
Hmmmmm I don't know if we have another place that tries to do that but sounds OK
There was a problem hiding this comment.
Actually, I just realized that InvalidArgumentRequiresSource is repetitive since there is already an InvalidArgumentRequiresSourceArgument message (which is also used several times throughout the file).
Removed HelpTableFormat message. The HelpTableFormatter needs to be updated to require LocalizedString as input. I intend to do this work in a separate PR.
locales/messages.en.json
Outdated
| "FailedToProvisionCe": "Failed to provision vcpkg-ce.", | ||
| "FailedToRunToolToDetermineVersion": "Failed to run {path} to determine the {tool_name} version.", | ||
| "FailedToStoreBackToMirror": "failed to store back to mirror:", | ||
| "FailedToStoreBinaryCache": "Failed to store binary cache '{path}':'{error_msg}'", | ||
| "FailedVendorAuthentication": "One or more {vendor} credential providers failed to authenticate. See '{url}' for more detailson how to provide credentials.", |
There was a problem hiding this comment.
| "FailedVendorAuthentication": "One or more {vendor} credential providers failed to authenticate. See '{url}' for more detailson how to provide credentials.", | |
| "FailedVendorAuthentication": "One or more {vendor} credential providers failed to authenticate. See '{url}' for more details on how to provide credentials.", |
There was a problem hiding this comment.
Note that the json files shouldn't be edited by hand: the C++ code that created these entries should be fixed and generate-messages rerun.
There was a problem hiding this comment.
Ah, yes. I've added the comments on the related code changes.
locales/messages.en.json
Outdated
| "RemovingPackage": "Removing {action_index}/{count} {spec}", | ||
| "RestoredPackagesFromVendor": "Restored {count} package(s) from {vendor} in {elapsed}", | ||
| "ReplaceSecretsError": "'{error_msg}'", |
There was a problem hiding this comment.
I don't agree with registering a localizable string just to surround it with '. Probably should use raw format here.
There was a problem hiding this comment.
I could just add a statement before the error message. Something like "Replace secretes produced the following error: '{error_msg}'." Is that cool with you or would you prefer to just write unlocalized to stdout?
There was a problem hiding this comment.
Yeah, we should add context to the message.
Co-authored-by: Victor Romero <[email protected]>
Co-authored-by: Victor Romero <[email protected]>
Co-authored-by: Victor Romero <[email protected]>
Co-authored-by: Victor Romero <[email protected]>
Co-authored-by: Victor Romero <[email protected]>
Co-authored-by: Victor Romero <[email protected]>
Localization of strings in binarycaching.cpp
Sorted Declared/Registered messages in alphabetical order.