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

Localize commands3 #642

Merged
merged 20 commits into from
Aug 18, 2022
Merged

Conversation

JavierMatosD
Copy link
Contributor

This PR localizes messages in the following files:

  • commands.format-manifest.cpp
  • commands.find.cpp
  • command.civerifyversions.cpp

I also added the function append_nl() which simply appends a newline character to the localized message. Previously, newline characters were added by using append_raw("\n").

@JavierMatosD JavierMatosD marked this pull request as ready for review July 20, 2022 21:38
@JavierMatosD JavierMatosD requested a review from BillyONeal July 20, 2022 21:39
# Conflicts:
#	include/vcpkg/base/messages.h
#	locales/messages.en.json
#	locales/messages.json
#	src/vcpkg/base/messages.cpp
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

This PR adds a lot of command-specific messages to the generic messages.h header. I recommend going through each of them and consider moving each one that's only used once into the respective .cpp. I see that this is now intended due to #615. Please disregard this review.

* Change the use of FailedToObtainLocalPortGitSha, FailedToWriteManifest, FailedToRemoveControl, FailedToFormatMissingFile to print the error prefix.
* Add missing : in FailedToParseJson.
* Delete now-unused JsonErrorFailedToParse and JsonErrorFailedToRead.
* Words nitpick on MissingArgFormatManifest (was preexisting).
* Change ControlAndManifestFilesPresent to check and add the error prefix rather than using a check form.
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.

4 participants