From 5341a2e4cd3b3774eab907b3dbe5aa32baf658a3 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 25 Feb 2022 17:15:34 -0800 Subject: [PATCH 1/3] Move LocalizedString out of namespace msg. The namespace msg is necessary for localized stuff to: 1. Give {formatter} parameters pretty short names without stepping on things. 2. To distinguish the localization safe versions and localization unsafe version of things e.g. vcpkg::println vs msg::println.println. LocalizedString doesn't fall into any of these categories, and as we start passing them around in more places adding msg:: is annoying, and encourages "using namespace"s that harm (1) and (2). --- include/vcpkg/base/fwd/messages.h | 6 +- include/vcpkg/base/messages.h | 137 +++++++++--------- include/vcpkg/base/parse.h | 12 +- include/vcpkg/commands.generate-message-map.h | 4 +- include/vcpkg/visualstudio.h | 2 +- src/vcpkg-test/messages.cpp | 4 +- src/vcpkg/base/messages.cpp | 15 +- src/vcpkg/base/parse.cpp | 16 +- src/vcpkg/commands.generate-message-map.cpp | 8 +- src/vcpkg/visualstudio.cpp | 8 +- 10 files changed, 105 insertions(+), 107 deletions(-) diff --git a/include/vcpkg/base/fwd/messages.h b/include/vcpkg/base/fwd/messages.h index 1c76f830fb..584199066f 100644 --- a/include/vcpkg/base/fwd/messages.h +++ b/include/vcpkg/base/fwd/messages.h @@ -1,5 +1,7 @@ #pragma once +#include + namespace vcpkg { #if defined(_WIN32) @@ -20,12 +22,10 @@ namespace vcpkg }; #endif - struct StringView; + struct LocalizedString; } namespace vcpkg::msg { - struct LocalizedString; - void write_unlocalized_text_to_stdout(Color c, vcpkg::StringView sv); } diff --git a/include/vcpkg/base/messages.h b/include/vcpkg/base/messages.h index 77b7a69785..a967ba95eb 100644 --- a/include/vcpkg/base/messages.h +++ b/include/vcpkg/base/messages.h @@ -10,6 +10,65 @@ #include +namespace vcpkg +{ + struct LocalizedString + { + LocalizedString() = default; + operator StringView() const { return m_data; } + const std::string& data() const { return m_data; } + std::string extract_data() { return std::exchange(m_data, ""); } + + static LocalizedString from_string_unchecked(std::string&& s) + { + LocalizedString res; + res.m_data = std::move(s); + return res; + } + + LocalizedString& append(const LocalizedString& s) + { + m_data.append(s.m_data); + return *this; + } + + LocalizedString& appendnl() + { + m_data.push_back('\n'); + return *this; + } + + friend const char* to_printf_arg(const LocalizedString& s) { return s.data().c_str(); } + + private: + std::string m_data; + + // to avoid lock-in on LocalizedString, these are free functions + // this allows us to convert to `std::string` in the future without changing All The Code + friend LocalizedString& append_newline(LocalizedString&); + friend LocalizedString&& append_newline(LocalizedString&& self) { return std::move(append_newline(self)); } + friend LocalizedString& appendnl(LocalizedString& self, const LocalizedString& to_append) + { + return append_newline(self.append(to_append)); + } + }; +} + +namespace fmt +{ + template<> + struct formatter : formatter + { + // parse is inherited from formatter + template + auto format(const vcpkg::LocalizedString& s, FormatContext& ctx) + { + return formatter::format(s.data(), ctx); + } + }; + +} + namespace vcpkg::msg { namespace detail @@ -61,55 +120,6 @@ namespace vcpkg::msg // initialize without any localized messages (use default messages only) void threadunsafe_initialize_context(); - struct LocalizedString - { - LocalizedString() = default; - operator StringView() const { return m_data; } - const std::string& data() const { return m_data; } - std::string extract_data() { return std::exchange(m_data, ""); } - - static LocalizedString from_string_unchecked(std::string&& s) - { - LocalizedString res; - res.m_data = std::move(s); - return res; - } - - LocalizedString& append(const LocalizedString& s) - { - m_data.append(s.m_data); - return *this; - } - LocalizedString& appendnl() - { - m_data.push_back('\n'); - return *this; - } - - private: - std::string m_data; - - // to avoid lock-in on LocalizedString, these are free functions - // this allows us to convert to `std::string` in the future without changing All The Code - friend LocalizedString& append_newline(LocalizedString&); - friend LocalizedString&& append_newline(LocalizedString&& self) { return std::move(append_newline(self)); } - friend LocalizedString& appendnl(LocalizedString& self, const LocalizedString& to_append) - { - return append_newline(self.append(to_append)); - } - }; - - inline const char* to_printf_arg(const msg::LocalizedString& s) { return s.data().c_str(); } - - struct LocalizedStringMapLess - { - using is_transparent = void; - bool operator()(const LocalizedString& lhs, const LocalizedString& rhs) const - { - return lhs.data() < rhs.data(); - } - }; - template LocalizedString format(Message, detail::MessageArgument... args) { @@ -120,19 +130,19 @@ namespace vcpkg::msg fmt::make_format_args(fmt::arg(Tags::name(), *args.parameter)...)); } - inline void println() { write_unlocalized_text_to_stdout(Color::none, "\n"); } + inline void println() { msg::write_unlocalized_text_to_stdout(Color::none, "\n"); } - inline void print(Color c, const LocalizedString& s) { write_unlocalized_text_to_stdout(c, s); } - inline void print(const LocalizedString& s) { write_unlocalized_text_to_stdout(Color::none, s); } + inline void print(Color c, const LocalizedString& s) { msg::write_unlocalized_text_to_stdout(c, s); } + inline void print(const LocalizedString& s) { msg::write_unlocalized_text_to_stdout(Color::none, s); } inline void println(Color c, const LocalizedString& s) { - write_unlocalized_text_to_stdout(c, s); - write_unlocalized_text_to_stdout(Color::none, "\n"); + msg::write_unlocalized_text_to_stdout(c, s); + msg::write_unlocalized_text_to_stdout(Color::none, "\n"); } inline void println(const LocalizedString& s) { - write_unlocalized_text_to_stdout(Color::none, s); - write_unlocalized_text_to_stdout(Color::none, "\n"); + msg::write_unlocalized_text_to_stdout(Color::none, s); + msg::write_unlocalized_text_to_stdout(Color::none, "\n"); } template @@ -233,18 +243,3 @@ namespace vcpkg::msg "", "error: cannot specify both --no-{option} and --{option}."); } - -namespace fmt -{ - template<> - struct formatter : formatter - { - // parse is inherited from formatter - template - auto format(const vcpkg::msg::LocalizedString& s, FormatContext& ctx) - { - return formatter::format(s.data(), ctx); - } - }; - -} diff --git a/include/vcpkg/base/parse.h b/include/vcpkg/base/parse.h index 9cbd890b7e..a9364ee905 100644 --- a/include/vcpkg/base/parse.h +++ b/include/vcpkg/base/parse.h @@ -60,9 +60,9 @@ namespace vcpkg::Parse struct ParseMessage { SourceLoc location = {}; - msg::LocalizedString message; + LocalizedString message; - msg::LocalizedString format(StringView origin, MessageKind kind) const; + LocalizedString format(StringView origin, MessageKind kind) const; }; struct ParseMessages @@ -131,11 +131,11 @@ namespace vcpkg::Parse void add_error(std::string message) { add_error(std::move(message), cur_loc()); } void add_error(std::string message, const SourceLoc& loc); - void add_error(msg::LocalizedString&& message) { add_error(message.extract_data(), cur_loc()); } - void add_error(msg::LocalizedString&& message, const SourceLoc& loc) { add_error(message.extract_data(), loc); } + void add_error(LocalizedString&& message) { add_error(message.extract_data(), cur_loc()); } + void add_error(LocalizedString&& message, const SourceLoc& loc) { add_error(message.extract_data(), loc); } - void add_warning(msg::LocalizedString&& message) { add_warning(std::move(message), cur_loc()); } - void add_warning(msg::LocalizedString&& message, const SourceLoc& loc); + void add_warning(LocalizedString&& message) { add_warning(std::move(message), cur_loc()); } + void add_warning(LocalizedString&& message, const SourceLoc& loc); const IParseError* get_error() const { return m_messages.error.get(); } std::unique_ptr extract_error() { return std::move(m_messages.error); } diff --git a/include/vcpkg/commands.generate-message-map.h b/include/vcpkg/commands.generate-message-map.h index 9c3162acb4..a96d727667 100644 --- a/include/vcpkg/commands.generate-message-map.h +++ b/include/vcpkg/commands.generate-message-map.h @@ -12,8 +12,8 @@ namespace vcpkg::Commands std::vector comments_without_argument; }; - std::vector get_all_format_args(StringView fstring, msg::LocalizedString& error); - FormatArgMismatches get_format_arg_mismatches(StringView value, StringView comment, msg::LocalizedString& error); + std::vector get_all_format_args(StringView fstring, LocalizedString& error); + FormatArgMismatches get_format_arg_mismatches(StringView value, StringView comment, LocalizedString& error); struct GenerateDefaultMessageMapCommand : BasicCommand { diff --git a/include/vcpkg/visualstudio.h b/include/vcpkg/visualstudio.h index 45e617e171..82b3c2fefd 100644 --- a/include/vcpkg/visualstudio.h +++ b/include/vcpkg/visualstudio.h @@ -16,7 +16,7 @@ namespace vcpkg #if defined(_WIN32) std::vector paths_examined; std::vector excluded_toolsets; - msg::LocalizedString get_localized_debug_info() const; + LocalizedString get_localized_debug_info() const; #endif }; } diff --git a/src/vcpkg-test/messages.cpp b/src/vcpkg-test/messages.cpp index e488f74933..0d094338d5 100644 --- a/src/vcpkg-test/messages.cpp +++ b/src/vcpkg-test/messages.cpp @@ -9,7 +9,7 @@ using namespace vcpkg::Commands; TEST_CASE ("generate message get_all_format_args", "[messages]") { - msg::LocalizedString err; + LocalizedString err; auto res = get_all_format_args("hey ho let's go", err); CHECK(err.data() == ""); CHECK(res == std::vector{}); @@ -37,7 +37,7 @@ TEST_CASE ("generate message get_all_format_args", "[messages]") TEST_CASE ("generate message get_format_arg_mismatches", "[messages]") { - msg::LocalizedString err; + LocalizedString err; auto res = get_format_arg_mismatches("hey ho", "", err); CHECK(err.data() == ""); CHECK(res.arguments_without_comment == std::vector{}); diff --git a/src/vcpkg/base/messages.cpp b/src/vcpkg/base/messages.cpp index b5e011c693..c4ef6b8d80 100644 --- a/src/vcpkg/base/messages.cpp +++ b/src/vcpkg/base/messages.cpp @@ -2,6 +2,15 @@ #include #include +namespace vcpkg +{ + LocalizedString& append_newline(LocalizedString& s) + { + s.m_data.push_back('\n'); + return s; + } +} + namespace vcpkg::msg { DECLARE_AND_REGISTER_MESSAGE(NoLocalizationForMessages, (), "", "No localization for the following messages:"); @@ -152,12 +161,6 @@ namespace vcpkg::msg } } - LocalizedString& append_newline(LocalizedString& s) - { - s.m_data.push_back('\n'); - return s; - } - void threadunsafe_initialize_context() { Messages& m = messages(); diff --git a/src/vcpkg/base/parse.cpp b/src/vcpkg/base/parse.cpp index cd38e44b33..6ff81643dd 100644 --- a/src/vcpkg/base/parse.cpp +++ b/src/vcpkg/base/parse.cpp @@ -30,7 +30,7 @@ namespace vcpkg::Parse auto decoder = Unicode::Utf8Decoder(line.data(), line.data() + line.size()); ParseMessage as_message; as_message.location = SourceLoc{std::next(decoder, caret_col), decoder, row, column}; - as_message.message = msg::LocalizedString::from_string_unchecked(std::string(message)); + as_message.message = LocalizedString::from_string_unchecked(std::string(message)); auto res = as_message.format(origin, MessageKind::Error).extract_data(); res.push_back('\n'); @@ -46,12 +46,12 @@ namespace vcpkg::Parse "Example of {value} is 'x64 & windows'", " on expression: {value}"); - msg::LocalizedString ParseMessage::format(StringView origin, MessageKind kind) const + LocalizedString ParseMessage::format(StringView origin, MessageKind kind) const { - msg::LocalizedString res = msg::format(msgFormattedParseMessageLocation, - msg::path = origin, - msg::row = location.row, - msg::column = location.column); + LocalizedString res = msg::format(msgFormattedParseMessageLocation, + msg::path = origin, + msg::row = location.row, + msg::column = location.column); if (kind == MessageKind::Warning) { res.append(msg::format(msg::msgWarningMessage)); @@ -88,7 +88,7 @@ namespace vcpkg::Parse } caret_string.push_back('^'); - res.append(msg::LocalizedString::from_string_unchecked(std::move(caret_string))); + res.append(LocalizedString::from_string_unchecked(std::move(caret_string))); return res; } @@ -128,7 +128,7 @@ namespace vcpkg::Parse return cur(); } - void ParserBase::add_warning(msg::LocalizedString&& message, const SourceLoc& loc) + void ParserBase::add_warning(LocalizedString&& message, const SourceLoc& loc) { m_messages.warnings.push_back(ParseMessage{loc, std::move(message)}); } diff --git a/src/vcpkg/commands.generate-message-map.cpp b/src/vcpkg/commands.generate-message-map.cpp index 602ae2a1a9..53ac1228de 100644 --- a/src/vcpkg/commands.generate-message-map.cpp +++ b/src/vcpkg/commands.generate-message-map.cpp @@ -58,7 +58,7 @@ namespace vcpkg::Commands nullptr, }; - std::vector get_all_format_args(StringView fstring, msg::LocalizedString& error) + std::vector get_all_format_args(StringView fstring, LocalizedString& error) { error = {}; std::vector res; @@ -117,7 +117,7 @@ namespace vcpkg::Commands return res; } - FormatArgMismatches get_format_arg_mismatches(StringView value, StringView comment, msg::LocalizedString& error) + FormatArgMismatches get_format_arg_mismatches(StringView value, StringView comment, LocalizedString& error) { FormatArgMismatches res; @@ -172,7 +172,7 @@ namespace vcpkg::Commands bool allow_bad_comments = !Util::Sets::contains(parsed_args.switches, OPTION_NO_ALLOW_BAD_COMMENTS); - msg::LocalizedString comments_msg_type; + LocalizedString comments_msg_type; Color comments_msg_color; if (allow_bad_comments) { @@ -222,7 +222,7 @@ namespace vcpkg::Commands std::sort(messages.begin(), messages.end(), MessageSorter{}); bool has_incorrect_comment = false; - msg::LocalizedString format_string_parsing_error; + LocalizedString format_string_parsing_error; Json::Object obj; for (Message& msg : messages) { diff --git a/src/vcpkg/visualstudio.cpp b/src/vcpkg/visualstudio.cpp index 5aacde31b4..6aa16ad56b 100644 --- a/src/vcpkg/visualstudio.cpp +++ b/src/vcpkg/visualstudio.cpp @@ -365,9 +365,9 @@ namespace vcpkg::VisualStudio } namespace vcpkg { - msg::LocalizedString ToolsetsInformation::get_localized_debug_info() const + LocalizedString ToolsetsInformation::get_localized_debug_info() const { - msg::LocalizedString ret; + LocalizedString ret; if (toolsets.empty()) { @@ -378,7 +378,7 @@ namespace vcpkg ret.append(msg::format(msgVSExaminedInstances)).appendnl(); for (const Toolset& toolset : toolsets) { - ret.append(msg::LocalizedString::from_string_unchecked( + ret.append(LocalizedString::from_string_unchecked( Strings::concat(" ", toolset.visual_studio_root_path, '\n'))); } } @@ -388,7 +388,7 @@ namespace vcpkg ret.append(msg::format(msgVSExaminedPaths)).appendnl(); for (const Path& examinee : paths_examined) { - ret.append(msg::LocalizedString::from_string_unchecked(Strings::concat(" ", examinee, '\n'))); + ret.append(LocalizedString::from_string_unchecked(Strings::concat(" ", examinee, '\n'))); } } From 33134cf4b739f56bc2fd2d014bd9fd118ae7bf29 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 25 Feb 2022 17:18:14 -0800 Subject: [PATCH 2/3] Teach Catch2 about LocalizedString. --- include/vcpkg-test/util.h | 10 ++++++++++ include/vcpkg/base/messages.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/include/vcpkg-test/util.h b/include/vcpkg-test/util.h index 5a4b9c7070..d9698ded70 100644 --- a/include/vcpkg-test/util.h +++ b/include/vcpkg-test/util.h @@ -4,6 +4,7 @@ #include #include +#include #include @@ -47,6 +48,15 @@ namespace Catch { static const std::string& convert(const vcpkg::Triplet& triplet) { return triplet.canonical_name(); } }; + + template<> + struct StringMaker + { + static const std::string convert(const vcpkg::LocalizedString& value) + { + return "LL\"" + value.data() + "\""; + } + }; } namespace vcpkg::Test diff --git a/include/vcpkg/base/messages.h b/include/vcpkg/base/messages.h index a967ba95eb..57e92abdcb 100644 --- a/include/vcpkg/base/messages.h +++ b/include/vcpkg/base/messages.h @@ -40,6 +40,36 @@ namespace vcpkg friend const char* to_printf_arg(const LocalizedString& s) { return s.data().c_str(); } + friend bool operator==(const LocalizedString& lhs, const LocalizedString& rhs) + { + return lhs.data() == rhs.data(); + } + + friend bool operator!=(const LocalizedString& lhs, const LocalizedString& rhs) + { + return lhs.data() != rhs.data(); + } + + friend bool operator<(const LocalizedString& lhs, const LocalizedString& rhs) + { + return lhs.data() < rhs.data(); + } + + friend bool operator<=(const LocalizedString& lhs, const LocalizedString& rhs) + { + return lhs.data() <= rhs.data(); + } + + friend bool operator>(const LocalizedString& lhs, const LocalizedString& rhs) + { + return lhs.data() > rhs.data(); + } + + friend bool operator>=(const LocalizedString& lhs, const LocalizedString& rhs) + { + return lhs.data() >= rhs.data(); + } + private: std::string m_data; From a5f8eed7ab0a0a97ec471ab87f179421fad09db4 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 28 Feb 2022 12:14:15 -0800 Subject: [PATCH 3/3] clang-format --- include/vcpkg-test/util.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/include/vcpkg-test/util.h b/include/vcpkg-test/util.h index d9698ded70..2f2a22238a 100644 --- a/include/vcpkg-test/util.h +++ b/include/vcpkg-test/util.h @@ -3,8 +3,8 @@ #include #include -#include #include +#include #include @@ -52,10 +52,7 @@ namespace Catch template<> struct StringMaker { - static const std::string convert(const vcpkg::LocalizedString& value) - { - return "LL\"" + value.data() + "\""; - } + static const std::string convert(const vcpkg::LocalizedString& value) { return "LL\"" + value.data() + "\""; } }; }