Skip to content

Commit

Permalink
Fix localization irregularities. (#553)
Browse files Browse the repository at this point in the history 
* Delete LocalizedString::appendnl.

I believe appendnl hails from a time when we thought we were going to be much more rigid about manipulation of LocalizedStrings as tokens. There are localized strings which themselves contain newlines. There are also formatted data strings, such as error message blocks join'd with \n, that we need to insert into localized messages. We also aren't likely to ever want to change our newlines to \r\n or something like that :)

To that end, I think both original reasons for this to exist are gone, and it should go away.

I think we should go all the way to removal rather than only removing the guidelines about it, because I think we, and contributors, will be confused if it exists and is relatively meaningless in the same way that people are confused (and use) std::endl today. The fact is that newlines aren't special in our LocalizedString structures, and we should be honest about that in our design.

Contrast/note: append_indent must stay, and we must add a plain std::string version of that, to ensure consistency in how we handle indenting, as this is likely to be actually ambiguous (as opposed to newlines which are always and forever \n in vcpkg).

* Improve formatting of generated comments, and deduplicate code that generates comments.

* Remove extra newlines from localized messages that are being fed into println or similar components that already insert newlines.

* Fix indents in "VcpkgHasCrashed" message. Also direct people to GitHub rather than email.

* Also fix UnsupportedToolchain message.

* Also fix VersionSpecMismatch message.

* Deduplicate all Error:s in localized messages.

* Add enforcement for not including indenting, error:, or warning: in localized messages, and fix remaining edge case occurrences. Also removes allow-incorrect-comments from generate-message-map considering that we control all inputs here and extending this to block these other lints seemed unnecessary.

* Also block trailing newlines.

* Fix end to end test.

* Rename print_error and print_warning to println_error and println_warning, respectively.
  • Loading branch information
@BillyONeal
BillyONeal authored May 24, 2022
1 parent ec42d25 commit 4ba70e9
Show file tree
Hide file tree
Showing 27 changed files with 489 additions and 452 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ endif()
# === Target: generate-message-map ===

add_custom_target(generate-message-map
COMMAND vcpkg x-generate-default-message-map --no-allow-incorrect-comments locales/messages.json
COMMAND vcpkg x-generate-default-message-map locales/messages.json
COMMAND vcpkg x-generate-default-message-map --no-output-comments locales/messages.en.json
WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
COMMENT "Update locales/messages.json"
Expand Down
8 changes: 4 additions & 4 deletions azure-pipelines/end-to-end-tests-dir/versions.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Throw-IfFailed
$CurrentTest = "default baseline"
$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root=$versionFilesPath/default-baseline-1 2>&1 | Out-String
Throw-IfNotFailed
if ($out -notmatch ".*Error: while checking out baseline\.*")
if ($out -notmatch ".*error: while checking out baseline\.*")
{
$out
throw "Expected to fail due to missing baseline"
Expand All @@ -80,9 +80,9 @@ if ($out -notmatch ".*Error: while checking out baseline\.*")
$CurrentTest = "mismatched version database"
$out = Run-Vcpkg @commonArgs "--feature-flags=versions" install --x-manifest-root="$PSScriptRoot/../e2e_ports/mismatched-version-database" 2>&1 | Out-String
Throw-IfNotFailed
if (($out -notmatch ".*Error: Failed to load port because version specs did not match*") -or
($out -notmatch ".*Expected: arrow@6.0.0.20210925#4.*") -or
($out -notmatch ".*Actual: arrow@6.0.0.20210925.*"))
if (($out -notmatch ".*error: Failed to load port because versions are inconsistent*") -or
($out -notmatch ".*version database indicates that it should be arrow@6.0.0.20210925#4.*") -or
($out -notmatch ".*contains the version arrow@6.0.0.20210925.*"))
{
$out
throw "Expected to fail due to mismatched versions between portfile and the version database"
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines/pipelines.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ jobs:
displayName: 'Generate the Messages File'
inputs:
script: |
build.x86.debug\vcpkg.exe x-generate-default-message-map --no-allow-incorrect-comments locales/messages.json
build.x86.debug\vcpkg.exe x-generate-default-message-map locales/messages.json
build.x86.debug\vcpkg.exe x-generate-default-message-map --no-output-comments locales/messages.en.json
- task: Powershell@2
displayName: 'Create Diff'
Expand Down
5 changes: 5 additions & 0 deletions include/vcpkg/base/checks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ namespace vcpkg::Checks
msg_exit_with_message(line_info, msg::format(m, args...));
}

[[noreturn]] inline void msg_exit_with_error(const LineInfo& line_info, const LocalizedString& message)
{
msg_exit_with_message(line_info, msg::format(msg::msgErrorMessage).append(message));
}

template<class Message, class... Args>
[[noreturn]] typename Message::is_message_type msg_exit_with_error(const LineInfo& line_info,
Message m,
Expand Down
73 changes: 35 additions & 38 deletions include/vcpkg/base/messages.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,26 @@ namespace vcpkg::msg
LocalizedString internal_vformat(::size_t index, fmt::format_args args);

template<class... Args>
MessageCheckFormatArgs<Args...> make_message_check_format_args(const Args&... args);
MessageCheckFormatArgs<Args...> make_message_check_format_args(const Args&... args); // not defined

template<class... Args>
std::string get_examples_for_args(StringView extra, const MessageCheckFormatArgs<Args...>&)
struct FormatArgAbi
{
const char* name;
const char* example;
};

std::string format_examples_for_args(StringView extra_comment, const FormatArgAbi* args, std::size_t arg_count);

inline std::string get_examples_for_args(StringView extra_comment, const MessageCheckFormatArgs<>&)
{
return extra_comment.to_string();
}

template<class Arg0, class... Args>
std::string get_examples_for_args(StringView extra_comment, const MessageCheckFormatArgs<Arg0, Args...>&)
{
std::string res(extra.begin(), extra.end());
if (!res.empty()) res.push_back('\n');
(void)(..., res.append(Args::comment()));
return res;
FormatArgAbi abi[] = {FormatArgAbi{Arg0::name, Arg0::example}, FormatArgAbi{Args::name, Args::example}...};
return format_examples_for_args(extra_comment, abi, 1 + sizeof...(Args));
}

::size_t startup_register_message(StringLiteral name, StringLiteral format_string, std::string&& comment);
Expand All @@ -179,11 +190,11 @@ namespace vcpkg::msg
template<class Message, class... Tags, class... Ts>
LocalizedString format(Message, detail::MessageArgument<Tags, Ts>... args)
{
// avoid generating code, but still typeck
// avoid generating code, but still typecheck
// (and avoid unused typedef warnings)
static_assert((Message::check_format_args((Tags{})...), true), "");
return detail::internal_vformat(Message::index,
fmt::make_format_args(fmt::arg(Tags::name(), *args.parameter)...));
fmt::make_format_args(fmt::arg(Tags::name, *args.parameter)...));
}

inline void println() { msg::write_unlocalized_text_to_stdout(Color::none, "\n"); }
Expand Down Expand Up @@ -228,11 +239,8 @@ namespace vcpkg::msg
#define DECLARE_MSG_ARG(NAME, EXAMPLE) \
constexpr static struct NAME##_t \
{ \
constexpr static const char* name() { return #NAME; } \
constexpr static const char* comment() \
{ \
return sizeof(EXAMPLE) > 1 ? "example of {" #NAME "} is '" EXAMPLE "'.\n" : ""; \
} \
constexpr static const char* name = #NAME; \
constexpr static const char* example = EXAMPLE; \
template<class T> \
detail::MessageArgument<NAME##_t, T> operator=(const T& t) const noexcept \
{ \
Expand All @@ -259,7 +267,6 @@ namespace vcpkg::msg
DECLARE_MSG_ARG(command_name, "install");
DECLARE_MSG_ARG(count, "42");
DECLARE_MSG_ARG(elapsed, "3.532 min");
DECLARE_MSG_ARG(email, "vcpkg@microsoft.com");
DECLARE_MSG_ARG(error_msg, "File Not Found");
DECLARE_MSG_ARG(exit_code, "127");
DECLARE_MSG_ARG(expected_version, "1.3.8");
Expand All @@ -281,24 +288,20 @@ namespace vcpkg::msg
DECLARE_MSG_ARG(env_var, "VCPKG_DEFAULT_TRIPLET");
#undef DECLARE_MSG_ARG

// These are `...` instead of
#define DECLARE_MESSAGE(NAME, ARGS, COMMENT, ...) \
constexpr struct NAME##_msg_t : decltype(::vcpkg::msg::detail::make_message_check_format_args ARGS) \
{ \
using is_message_type = void; \
static ::vcpkg::StringLiteral name() { return #NAME; } \
static ::vcpkg::StringLiteral extra_comment() { return COMMENT; } \
static ::std::string full_comment(); \
static ::vcpkg::StringLiteral default_format_string() noexcept { return __VA_ARGS__; } \
static constexpr ::vcpkg::StringLiteral name = #NAME; \
static constexpr ::vcpkg::StringLiteral extra_comment = COMMENT; \
static constexpr ::vcpkg::StringLiteral default_format_string = __VA_ARGS__; \
static const ::size_t index; \
} msg##NAME VCPKG_UNUSED = {}
#define REGISTER_MESSAGE(NAME) \
::std::string NAME##_msg_t::full_comment() \
{ \
return ::vcpkg::msg::detail::get_examples_for_args(NAME##_msg_t::extra_comment(), NAME##_msg_t{}); \
} \
const ::size_t NAME##_msg_t::index = ::vcpkg::msg::detail::startup_register_message( \
NAME##_msg_t::name(), NAME##_msg_t::default_format_string(), NAME##_msg_t::full_comment())
NAME##_msg_t::name, \
NAME##_msg_t::default_format_string, \
::vcpkg::msg::detail::get_examples_for_args(NAME##_msg_t::extra_comment, NAME##_msg_t{}))
#define DECLARE_AND_REGISTER_MESSAGE(NAME, ARGS, COMMENT, ...) \
DECLARE_MESSAGE(NAME, ARGS, COMMENT, __VA_ARGS__); \
REGISTER_MESSAGE(NAME)
Expand All @@ -318,25 +321,19 @@ namespace vcpkg::msg
DECLARE_MESSAGE(BothYesAndNoOptionSpecifiedError,
(msg::option),
"",
"error: cannot specify both --no-{option} and --{option}.");
"cannot specify both --no-{option} and --{option}.");
inline void print_warning(const LocalizedString& s)
{
print(Color::warning, format(msgWarningMessage).append(s).append_raw('\n'));
}
void println_warning(const LocalizedString& s);
template<class Message, class... Ts>
typename Message::is_message_type print_warning(Message m, Ts... args)
typename Message::is_message_type println_warning(Message m, Ts... args)
{
print(Color::warning, format(msgWarningMessage).append(format(m, args...).append_raw('\n')));
println_warning(format(m, args...));
}
inline void print_error(const LocalizedString& s)
{
print(Color::error, format(msgErrorMessage).append(s).append_raw('\n'));
}
void println_error(const LocalizedString& s);
template<class Message, class... Ts>
typename Message::is_message_type print_error(Message m, Ts... args)
typename Message::is_message_type println_error(Message m, Ts... args)
{
print(Color::error, format(msgErrorMessage).append(format(m, args...).append_raw('\n')));
println_error(format(m, args...));
}
}
1 change: 0 additions & 1 deletion include/vcpkg/commands.contact.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace vcpkg::Commands::Contact
{
extern const CommandStructure COMMAND_STRUCTURE;
const std::string& email();
void perform_and_exit(const VcpkgCmdArguments& args, Filesystem& fs);

struct ContactCommand : BasicCommand
Expand Down
Loading

0 comments on commit 4ba70e9

Please sign in to comment.