Delete LocalizedString::appendnl. (#547)

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).

docs/localization.md

@@ -67,7 +67,6 @@ Messages in vcpkg are written in American English. They should not contain:
 * formatting:
   - indentation should be added with the `append_indent()` function;
     if you need to add more than one indentation, you can use `append_indent(N)`
-  - newlines should be added with the `appendnl()` function
   - Any other interesting characters (like `- ` for lists, for example) should use `append_raw(...)`
 * or for the prefixes:
   - `"warning: "`, instead use `msg::format(msg::msgWarningMessage).append(msgMyWarning)`

include/vcpkg/base/messages.h

@@ -40,7 +40,11 @@ namespace vcpkg
         {
             return LocalizedString(StringView(s));
         }
-
+        LocalizedString& append_raw(char c)
+        {
+            m_data.push_back(c);
+            return *this;
+        }
         LocalizedString& append_raw(StringView s)
         {
             m_data.append(s.begin(), s.size());
@@ -63,11 +67,6 @@ namespace vcpkg
             return append(msg::format(m, args...));
         }
 
-        LocalizedString& appendnl()
-        {
-            m_data.push_back('\n');
-            return *this;
-        }
         LocalizedString& append_indent(size_t indent = 1)
         {
             m_data.append(indent * 4, ' ');
@@ -210,7 +209,7 @@ namespace vcpkg::msg
     template<class Message, class... Ts>
     void println(Message m, Ts... args)
     {
-        print(format(m, args...).appendnl());
+        print(format(m, args...).append_raw('\n'));
     }
 
     template<class Message, class... Ts>
@@ -221,7 +220,7 @@ namespace vcpkg::msg
     template<class Message, class... Ts>
     void println(Color c, Message m, Ts... args)
     {
-        print(c, format(m, args...).appendnl());
+        print(c, format(m, args...).append_raw('\n'));
     }
 
 // these use `constexpr static` instead of `inline` in order to work with GCC 6;
@@ -323,21 +322,21 @@ namespace vcpkg::msg
 
     inline void print_warning(const LocalizedString& s)
     {
-        print(Color::warning, format(msgWarningMessage).append(s).appendnl());
+        print(Color::warning, format(msgWarningMessage).append(s).append_raw('\n'));
     }
     template<class Message, class... Ts>
     typename Message::is_message_type print_warning(Message m, Ts... args)
     {
-        print(Color::warning, format(msgWarningMessage).append(format(m, args...).appendnl()));
+        print(Color::warning, format(msgWarningMessage).append(format(m, args...).append_raw('\n')));
     }
 
     inline void print_error(const LocalizedString& s)
     {
-        print(Color::error, format(msgErrorMessage).append(s).appendnl());
+        print(Color::error, format(msgErrorMessage).append(s).append_raw('\n'));
     }
     template<class Message, class... Ts>
     typename Message::is_message_type print_error(Message m, Ts... args)
     {
-        print(Color::error, format(msgErrorMessage).append(format(m, args...).appendnl()));
+        print(Color::error, format(msgErrorMessage).append(format(m, args...).append_raw('\n')));
     }
 }

src/vcpkg-fuzz/main.cpp

@@ -81,7 +81,7 @@ namespace
                     else
                     {
                         msg::print_error(msg::format(msgFuzzInvalidKind, msg::value = value)
-                                             .appendnl()
+                                             .append_raw('\n')
                                              .append_indent()
                                              .append(msgFuzzExpectedOneOf));
                         print_help_and_exit(true);
@@ -115,9 +115,9 @@ namespace
         {
             auto color = invalid ? Color::error : Color::none;
 
-            auto message = msg::format(msgFuzzHelpUsage).appendnl().appendnl();
-            message.append(msgFuzzHelpInput).appendnl().appendnl();
-            message.append(msgFuzzHelpOptions).appendnl();
+            auto message = msg::format(msgFuzzHelpUsage).append_raw("\n\n");
+            message.append(msgFuzzHelpInput).append_raw("\n\n");
+            message.append(msgFuzzHelpOptions).append_raw('\n');
 
             struct
             {
@@ -133,7 +133,7 @@ namespace
                 message.append_raw(start_option)
                     .append_raw(std::string(30 - start_option.size(), ' '))
                     .append(option.help)
-                    .appendnl();
+                    .append_raw('\n');
             }
 
             msg::print(color, message);

src/vcpkg.cpp

@@ -343,15 +343,17 @@ int main(const int argc, const char* const* const argv)
     fflush(stdout);
     msg::println();
     LocalizedString data_blob;
-    data_blob.append_raw("Version=").append_raw(Commands::Version::version).appendnl();
-    data_blob.append_raw("EXCEPTION=").append_raw(exc_msg).appendnl();
-    data_blob.append_raw("CMD=").appendnl();
+    data_blob.append_raw("Version=")
+        .append_raw(Commands::Version::version)
+        .append_raw("\nEXCEPTION=")
+        .append_raw(exc_msg)
+        .append_raw("\nCMD=\n");
     for (int x = 0; x < argc; ++x)
     {
 #if defined(_WIN32)
-        data_blob.append_raw(Strings::to_utf8(argv[x])).append_raw("|").appendnl();
+        data_blob.append_raw(Strings::to_utf8(argv[x])).append_raw("|\n");
 #else
-        data_blob.append_raw(argv[x]).append_raw("|").appendnl();
+        data_blob.append_raw(argv[x]).append_raw("|\n");
 #endif
     }
 

src/vcpkg/archives.cpp

@@ -109,7 +109,7 @@ namespace
             Checks::msg_exit_with_message(
                 VCPKG_LINE_INFO,
                 msg::format(msgMsiexecFailedToExtract, msg::path = archive, msg::exit_code = code_and_output.exit_code)
-                    .appendnl()
+                    .append_raw('\n')
                     .append_raw(code_and_output.output));
         }
     }

src/vcpkg/base/checks.cpp

@@ -102,7 +102,7 @@ namespace vcpkg
                          msg::format(msg::msgInternalErrorMessage)
                              .append(locale_invariant_lineinfo(line_info))
                              .append(msgChecksFailedCheck)
-                             .appendnl()
+                             .append_raw('\n')
                              .append(msg::msgInternalErrorMessageContact));
             exit_fail(line_info);
         }

src/vcpkg/base/git.cpp

@@ -164,7 +164,7 @@ namespace vcpkg
         if (output.exit_code != 0)
         {
             return msg::format(msgGitCommandFailed, msg::command_line = cmd.command_line())
-                .appendnl()
+                .append_raw('\n')
                 .append_raw(output.output);
         }
         return parse_git_status_output(output.output);

src/vcpkg/base/parse.cpp

@@ -66,15 +66,15 @@ namespace vcpkg
         }
         res.append(message);
 
-        res.appendnl();
+        res.append_raw('\n');
 
         auto line_end = Util::find_if(location.it, ParserBase::is_lineend);
         StringView line = StringView{
             location.start_of_line.pointer_to_current(),
             line_end.pointer_to_current(),
         };
         res.append(msg::format(msgFormattedParseMessageExpression, msg::value = line));
-        res.appendnl();
+        res.append_raw('\n');
 
         auto caret_point = StringView{location.start_of_line.pointer_to_current(), location.it.pointer_to_current()};
         auto formatted_caret_point = msg::format(msgFormattedParseMessageExpression, msg::value = caret_point);

src/vcpkg/build.cpp

@@ -268,7 +268,7 @@ namespace vcpkg::Build
             LocalizedString errorMsg = msg::format(msg::msgErrorMessage).append(msgBuildDependenciesMissing);
             for (const auto& p : result.unmet_dependencies)
             {
-                errorMsg.append_indent().append_raw(p.to_string()).appendnl();
+                errorMsg.append_indent().append_raw(p.to_string()).append_raw('\n');
             }
 
             Checks::msg_exit_with_message(VCPKG_LINE_INFO, errorMsg);
@@ -281,7 +281,7 @@ namespace vcpkg::Build
             LocalizedString warnings;
             for (auto&& msg : action->build_failure_messages)
             {
-                warnings.append(msg).appendnl();
+                warnings.append(msg).append_raw('\n');
             }
             if (!warnings.data().empty())
             {
@@ -1548,34 +1548,34 @@ namespace vcpkg::Build
 
         if (build_result.code == BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES)
         {
-            res.appendnl().append_indent().append(msgBuildingPackageFailedDueToMissingDeps);
+            res.append_raw('\n').append_indent().append(msgBuildingPackageFailedDueToMissingDeps);
 
             for (const auto& missing_spec : build_result.unmet_dependencies)
             {
-                res.appendnl().append_indent(2).append_raw(missing_spec.to_string());
+                res.append_raw('\n').append_indent(2).append_raw(missing_spec.to_string());
             }
         }
 
-        res.appendnl();
+        res.append_raw('\n');
         return res;
     }
 
     LocalizedString create_user_troubleshooting_message(const InstallPlanAction& action, const VcpkgPaths& paths)
     {
         const auto& spec_name = action.spec.name();
-        LocalizedString result = msg::format(msgBuildTroubleshootingMessage1).appendnl();
+        LocalizedString result = msg::format(msgBuildTroubleshootingMessage1).append_raw('\n');
         result.append_indent()
             .append_raw("https://github.com/microsoft/vcpkg/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+")
             .append_raw(spec_name)
-            .appendnl();
-        result.append(msgBuildTroubleshootingMessage2).appendnl();
+            .append_raw('\n');
+        result.append(msgBuildTroubleshootingMessage2).append_raw('\n');
         result.append_indent()
             .append_fmt_raw("https://github.com/microsoft/vcpkg/issues/"
                             "new?template=report-package-build-failure.md&title=[{}]+Build+error",
                             spec_name)
-            .appendnl();
-        result.append(msgBuildTroubleshootingMessage3, msg::package_name = spec_name).appendnl();
-        result.append_raw(paths.get_toolver_diagnostics()).appendnl();
+            .append_raw('\n');
+        result.append(msgBuildTroubleshootingMessage3, msg::package_name = spec_name).append_raw('\n');
+        result.append_raw(paths.get_toolver_diagnostics()).append_raw('\n');
         return result;
     }
 

src/vcpkg/cmakevars.cpp

@@ -276,7 +276,7 @@ endfunction()
             Checks::msg_exit_with_message(
                 VCPKG_LINE_INFO,
                 msg::format(msgCommandFailed, msg::command_line = cmd_launch_cmake.command_line())
-                    .appendnl()
+                    .append_raw('\n')
                     .append_raw(Strings::join(", ", lines)));
         }
 

src/vcpkg/commands.add-version.cpp

@@ -328,14 +328,11 @@ namespace
                 msg::print_warning(msg::format(msgAddVersionPortFilesShaUnchanged,
                                                msg::package_name = port_name,
                                                msg::version = found_same_sha->first.version)
-                                       .appendnl()
-                                       .append_raw("-- SHA: ")
+                                       .append_raw("\n-- SHA: ")
                                        .append_raw(git_tree)
-                                       .appendnl()
-                                       .append_raw("-- ")
+                                       .append_raw("\n-- ")
                                        .append(msgAddVersionCommitChangesReminder)
-                                       .appendnl()
-                                       .append_raw("***")
+                                       .append_raw("\n***")
                                        .append(msgAddVersionNoFilesUpdated)
                                        .append_raw("***"));
                 if (keep_going) return UpdateResult::NotUpdated;
@@ -353,18 +350,17 @@ namespace
                 {
                     msg::print_error(
                         msg::format(msgAddVersionPortFilesShaChanged, msg::package_name = port_name)
-                            .appendnl()
+                            .append_raw('\n')
                             .append(msgAddVersionVersionIs, msg::version = port_version.version)
-                            .appendnl()
+                            .append_raw('\n')
                             .append(msgAddVersionOldShaIs, msg::value = it->second)
-                            .appendnl()
+                            .append_raw('\n')
                             .append(msgAddVersionNewShaIs, msg::value = git_tree)
-                            .appendnl()
+                            .append_raw('\n')
                             .append(msgAddVersionUpdateVersionReminder)
-                            .appendnl()
+                            .append_raw('\n')
                             .append(msgAddVersionOverwriteOptionSuggestion, msg::option = OPTION_OVERWRITE_VERSION)
-                            .appendnl()
-                            .append_raw("***")
+                            .append_raw("\n***")
                             .append(msgAddVersionNoFilesUpdated)
                             .append_raw("***"));
                     if (keep_going) return UpdateResult::NotUpdated;
@@ -396,7 +392,7 @@ namespace
         }
 
         msg::print_error(msg::format(msgAddVersionUnableToParseVersionsFile, msg::path = version_db_file_path)
-                             .appendnl()
+                             .append_raw('\n')
                              .append_raw(maybe_versions.error()));
         Checks::exit_fail(VCPKG_LINE_INFO);
     }
@@ -523,11 +519,11 @@ namespace vcpkg::Commands::AddVersion
                         auto command_line = fmt::format("vcpkg format-manifest ports/{}/vcpkg.json", port_name);
                         msg::print_error(
                             msg::format(msgAddVersionPortHasImproperFormat, msg::package_name = port_name)
-                                .appendnl()
+                                .append_raw('\n')
                                 .append(msgAddVersionFormatPortSuggestion, msg::command_line = command_line)
-                                .appendnl()
+                                .append_raw('\n')
                                 .append(msgAddVersionCommitResultReminder)
-                                .appendnl());
+                                .append_raw('\n'));
                         Checks::check_exit(VCPKG_LINE_INFO, !add_all);
                         continue;
                     }
@@ -546,11 +542,9 @@ namespace vcpkg::Commands::AddVersion
             if (git_tree_it == git_tree_map.end())
             {
                 msg::print_warning(msg::format(msgAddVersionNoGitSha, msg::package_name = port_name)
-                                       .appendnl()
-                                       .append_raw("-- ")
+                                       .append_raw("\n-- ")
                                        .append(msgAddVersionCommitChangesReminder)
-                                       .appendnl()
-                                       .append_raw("***")
+                                       .append_raw("\n***")
                                        .append(msgAddVersionNoFilesUpdated)
                                        .append_raw("***"));
                 if (add_all) continue;

src/vcpkg/commands.ci.cpp

@@ -510,7 +510,7 @@ namespace vcpkg::Commands::CI
                                                   msg::build_result =
                                                       Build::to_string_locale_invariant(build_result.code).to_string(),
                                                   msg::path = ci_baseline_file_name));
-                        output.appendnl();
+                        output.append_raw('\n');
                     }
                     break;
                 case Build::BuildResult::SUCCEEDED:
@@ -519,7 +519,7 @@ namespace vcpkg::Commands::CI
                         output.append(msg::format(msgCiBaselineUnexpectedPass,
                                                   msg::spec = port_result.get_spec().to_string(),
                                                   msg::path = ci_baseline_file_name));
-                        output.appendnl();
+                        output.append_raw('\n');
                     }
                     break;
                 default: break;
@@ -533,7 +533,7 @@ namespace vcpkg::Commands::CI
         }
 
         LocalizedString header = msg::format(msgCiBaselineRegressionHeader);
-        header.appendnl();
+        header.append_raw('\n');
         output_data.insert(0, header.extract_data());
         fwrite(output_data.data(), 1, output_data.size(), stderr);
     }

src/vcpkg/commands.update-baseline.cpp

@@ -80,7 +80,7 @@ namespace vcpkg::Commands
         // this isn't an error, since we want to continue attempting to update baselines
         msg::print_warning(
             msg::format(msgUpdateBaselineNoUpdate, msg::url = url, msg::value = reg.baseline.value_or(""))
-                .appendnl()
+                .append_raw('\n')
                 .append(new_baseline_res.error()));
     }
 

src/vcpkg/configuration.cpp

@@ -554,7 +554,7 @@ namespace vcpkg
         else
         {
             return msg::format(msgUpdateBaselineRemoteGitError, msg::url = url)
-                .appendnl()
+                .append_raw('\n')
                 .append_raw(Strings::trim(res.error()));
         }
     }
@@ -582,7 +582,7 @@ namespace vcpkg
                 else
                 {
                     return msg::format(msgUpdateBaselineLocalGitError, msg::path = paths.root)
-                        .appendnl()
+                        .append_raw('\n')
                         .append_raw(Strings::trim(res.error()));
                 }
             }