Localize ParseControlErrorInfo. #521
Conversation
There was a problem hiding this comment.
This does have significant functional impact on the output (it is now sorted by port instead of by error type), however I think this is an ok change.
Given that the only remaining reason for ParseControlErrorInfo to exist after this (compared to just using a LocalizedString) is to contain the single bit of "Was there a CONTROL file with extra fields?", we may want to consider simplifying the pathway and just linking users to online documentation.
|
After some discussion in person I'm not sure I'm happy with this change right now, so setting to draft. |
|
Unfortunately #516 damaged what this PR is trying to do beyond repair. |
Upon further reflection I think this is salvagable. |
# Conflicts: # include/vcpkg/paragraphparser.h # locales/messages.en.json # locales/messages.json # src/vcpkg/sourceparagraph.cpp
I tried to do what we discussed that led me to drafting this, eliminating ParseControlErrorInfo entirely. Unfortunately that change ends up being HUGE and I think this moves the needle in a positive direction, so undrafting. |
|
Upon reflection I think we should print everything in the error block in the error color rather than looking for "error:" as was added in #516 but I didn't change that for now. |
src/vcpkg/base/git.cpp
Outdated
| @@ -147,7 +147,7 @@ namespace vcpkg | |||
|
|
|||
| if (auto error = parser.get_error()) | |||
| { | |||
| return msg::format(msgGitUnexpectedCommandOutput).append_raw(error->format()); | |||
| return msg::format(msgGitUnexpectedCommandOutput).append_raw(error->to_string()); | |||
There was a problem hiding this comment.
| return msg::format(msgGitUnexpectedCommandOutput).append_raw(error->to_string()); | |
| return msg::format(msgGitUnexpectedCommandOutput).append_raw("\n").append_raw(error->to_string()); |
I think this needs a newline because error->to_string() expects to begin at start of line
There was a problem hiding this comment.
Also, it would be nice if append_raw() would eat anything that can be Strings::append()'d
Noticed that this is actually a smart pointer, so Strings::append() wouldn't recognize it anyway.
src/vcpkg/paragraphs.cpp
Outdated
| DECLARE_AND_REGISTER_MESSAGE(ParseControlErrorInfoWhileLoading, | ||
| (msg::path, msg::error_msg), | ||
| "", | ||
| "while loading {path}: {error_msg}"); |
There was a problem hiding this comment.
I think this one should not contain error_msg -- it should be appended at the call site. I believe error_msg is a multiline blob with headers, such as:
../path/to/file:10: error: identifier not valid
src/vcpkg/paragraphs.cpp
Outdated
| (msg::path), | ||
| "", | ||
| "there are invalid fields in the control or manifest file of {path}.\nThe following fields were not expected:"); | ||
| DECLARE_AND_REGISTER_MESSAGE(ParseControlErrorInfoEntry, (msg::path, msg::error_msg), "", "In {path}: {error_msg}"); |
There was a problem hiding this comment.
This isn't an error_msg, this is a list.
to_stringprotocol.ParseControlErrorInfo::format_errorswith that. Note that this is a behavior change, since errors are grouped by file rather than by error type.