refactor: split error message from source location in reports

fix #672

Author: alandefreitas

include/mrdocs/Support/Error.hpp

@@ -2775,7 +2775,62 @@ void
 print(
     Level level,
     std::string const& text,
-    source_location const* loc = nullptr);
+    source_location const* loc = nullptr,
+    Error const* e = nullptr);
+
+namespace detail {
+template<class Arg0, class... Args>
+requires (!std::same_as<std::decay_t<Arg0>, Error>)
+void
+log_impl(
+    Level level,
+    Located<std::string_view> fs,
+    Arg0&& arg0,
+    Args&&... args)
+{
+    std::string str = fmt::vformat(
+        fs.value,
+        fmt::make_format_args(arg0, args...));
+    return print(
+        level,
+        str,
+        &fs.where);
+}
+
+template<class... Args>
+void
+log_impl(
+    Level level,
+    Located<std::string_view> fs,
+    Error const& e,
+    Args&&... args)
+{
+    // When the message is an error, we send split
+    // the information relevant to the user from
+    // the information relevant for bug tracking
+    // so that users can understand the message.
+    std::string str = fmt::vformat(
+        fs.value,
+        fmt::make_format_args(e.reason(), args...));
+    return print(
+        level,
+        str,
+        &fs.where,
+        &e);
+}
+
+inline
+void
+log_impl(
+    Level level,
+    Located<std::string_view> fs)
+{
+    return print(
+        level,
+        {},
+        &fs.where);
+}
+}
 
 /** Format a message to the console.
 
@@ -2791,17 +2846,15 @@ print(
 */
 template<class... Args>
 void
-format(
+log(
     Level level,
     Located<std::string_view> fs,
     Args&&... args)
 {
-    return print(
+    return detail::log_impl(
         level,
-        fmt::vformat(
-            fs.value,
-            fmt::make_format_args(args...)),
-        &fs.where);
+        fs,
+        std::forward<Args>(args)...);
 }
 
 /** Report a message to the console.
@@ -2812,12 +2865,7 @@ debug(
     Located<std::string_view> format,
     Args&&... args)
 {
-    return print(
-        Level::debug,
-        fmt::vformat(
-            format.value,
-            fmt::make_format_args(args...)),
-        &format.where);
+    return log(Level::debug, format, std::forward<Args>(args)...);
 }
 
 /** Report a message to the console.
@@ -2828,12 +2876,7 @@ info(
     Located<std::string_view> format,
     Args&&... args)
 {
-    return print(
-        Level::info,
-        fmt::vformat(
-            format.value,
-            fmt::make_format_args(args...)),
-        &format.where);
+    return log(Level::info, format, std::forward<Args>(args)...);
 }
 
 /** Report a message to the console.
@@ -2844,12 +2887,7 @@ warn(
     Located<std::string_view> format,
     Args&&... args)
 {
-    return print(
-        Level::warn,
-        fmt::vformat(
-            format.value,
-            fmt::make_format_args(args...)),
-        &format.where);
+    return log(Level::warn, format, std::forward<Args>(args)...);
 }
 
 /** Report a message to the console.
@@ -2860,12 +2898,7 @@ error(
     Located<std::string_view> format,
     Args&&... args)
 {
-    return print(
-        Level::error,
-        fmt::vformat(
-            format.value,
-            fmt::make_format_args(args...)),
-        &format.where);
+    return log(Level::error, format, std::forward<Args>(args)...);
 }
 
 /** Report a message to the console.
@@ -2876,12 +2909,7 @@ fatal(
     Located<std::string_view> format,
     Args&&... args)
 {
-    return print(
-        Level::fatal,
-        fmt::vformat(
-            format.value,
-            fmt::make_format_args(args...)),
-        &format.where);
+    return log(Level::fatal, format, std::forward<Args>(args)...);
 }
 
 } // report

src/lib/Lib/CorpusImpl.cpp

@@ -187,9 +187,8 @@ build(
             taskGroup.async(
             [&, idx = ++index, path = std::move(file)]()
             {
-                report::format(reportLevel,
+                report::log(reportLevel,
                     "[{}/{}] \"{}\"", idx, files.size(), path);
-
                 processFile(path);
             });
         }
@@ -217,7 +216,7 @@ build(
         return Unexpected(results.error());
     corpus->info_ = std::move(results.value());
 
-    report::format(reportLevel,
+    report::log(reportLevel,
         "Extracted {} declarations in {}",
         corpus->info_.size(),
         format_duration(clock_type::now() - start_time));

src/lib/Support/Error.cpp

@@ -10,6 +10,7 @@
 
 #include "lib/Support/Error.hpp"
 #include <mrdocs/Support/Path.hpp>
+#include <mrdocs/Version.hpp>
 #include <llvm/Support/Mutex.h>
 #include <llvm/Support/raw_ostream.h>
 #include <llvm/Support/Signals.h>
@@ -201,13 +202,14 @@ void
 print(
     Level level,
     std::string const& text,
-    source_location const* loc)
+    source_location const* loc,
+    Error const* e)
 {
     call_impl(level,
         [&](llvm::raw_ostream& os)
         {
             os << text;
-        }, loc);
+        }, loc, e);
 }
 
 //------------------------------------------------
@@ -230,7 +232,8 @@ void
 call_impl(
     Level level,
     std::function<void(llvm::raw_ostream&)> f,
-    source_location const* loc)
+    source_location const* loc,
+    Error const* e)
 {
     std::string s;
     if(level >= level_)
@@ -242,11 +245,22 @@ call_impl(
             level == Level::error ||
             level == Level::fatal))
         {
-            os << "\n" <<
-                fmt::format(
-                    "    Reported at {}({})",
-                    ::SourceFileNames::getFileName(loc->file_name()),
-                    loc->line());
+            os << "\n\n";
+            os << "An issue occurred during execution.\n";
+            os << "If you believe this is a bug, please report it at https://github.com/cppalliance/mrdocs/issues\n"
+                  "with the following details:\n";
+            os << fmt::format("    MrDocs Version: {} (Build: {})\n", project_version, project_version_build);
+            if (e)
+            {
+                os << fmt::format(
+                    "    Error Location: `{}` at line {}\n",
+                    ::SourceFileNames::getFileName(e->location().file_name()),
+                    e->location().line());
+            }
+            os << fmt::format(
+                "    Reported From: `{}` at line {}",
+                ::SourceFileNames::getFileName(loc->file_name()),
+                loc->line());
             // VFALCO attach a stack trace for Level::fatal
         }
         os << '\n';

src/lib/Support/Error.hpp

@@ -119,7 +119,8 @@ void
 call_impl(
     Level level,
     std::function<void(llvm::raw_ostream&)> f,
-    source_location const* loc);
+    source_location const* loc,
+    Error const* e = nullptr);
 
 /** Formatted reporting to a live stream.
 

src/tool/ToolMain.cpp

@@ -135,7 +135,7 @@ mrdocs_main(int argc, char const** argv)
     auto exp = DoGenerateAction(configPath, dirs, argv);
     if (!exp)
     {
-        report::error("Generating reference failed: {}", exp.error().message());
+        report::error("Generating reference failed: {}", exp.error());
     }
     if (report::results.errorCount > 0 ||
         report::results.fatalCount > 0)

util/generate-config-info.py

@@ -105,14 +105,14 @@ def validate_and_normalize_option(option):
     if 'command-line-only' not in option:
         option['command-line-only'] = False
     if 'brief' not in option:
-        raise ValueError(f'Option {option["name"]} must have a "brief" description')
+        raise ValueError(f'Option "{option["name"]}" must have a "brief" description')
     if 'details' not in option:
         option['details'] = ''
     if 'type' not in option:
         option['type'] = 'string'
     if not is_valid_option_type(option['type']):
         raise ValueError(
-            f'Option {option["name"]} has an invalid type {option["type"]}: It should be one of {get_valid_option_values()}')
+            f'Option "{option["name"]}" has an invalid type {option["type"]}: It should be one of {get_valid_option_values()}')
     if option['type'] == 'enum':
         if 'values' not in option:
             raise ValueError(f'Option "{option["name"]}" is of type enum and must have "values"')
@@ -541,7 +541,7 @@ def option_validation_snippet(option):
                 validation_contents += f'// s.{camel_name} is required and has no default value\n'
                 validation_contents += f'if (s.{camel_name}.empty())\n'
                 validation_contents += f'{{\n'
-                validation_contents += f'    return Unexpected(formatError("{option["name"]} option is required"));'
+                validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option is required"));'
                 validation_contents += f'}}\n'
                 validation_contents += f'else\n'
                 validation_contents += f'{{\n'
@@ -592,30 +592,30 @@ def option_validation_snippet(option):
         if option['must-exist']:
             validation_contents += f'if (!s.{camel_name}.empty() && !files::exists(s.{camel_name}))\n'
             validation_contents += f'{{\n'
-            validation_contents += f'    return Unexpected(formatError("{option["name"]} option: path does not exist: {{}}", s.{camel_name}));\n'
+            validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: path does not exist: {{}}", s.{camel_name}));\n'
             validation_contents += f'}}\n'
             if option['type'] == 'file-path':
                 validation_contents += f'if (files::isDirectory(s.{camel_name}))\n'
                 validation_contents += f'{{\n'
-                validation_contents += f'    return Unexpected(formatError("{option["name"]} option: path should be a regular file: {{}}", s.{camel_name}));\n'
+                validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: path should be a regular file: {{}}", s.{camel_name}));\n'
                 validation_contents += f'}}\n'
             if option['type'] == 'dir-path':
                 validation_contents += f'if (!files::isDirectory(s.{camel_name}))\n'
                 validation_contents += f'{{\n'
-                validation_contents += f'    return Unexpected(formatError("{option["name"]} option: path should be a directory: {{}}", s.{camel_name}));\n'
+                validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: path should be a directory: {{}}", s.{camel_name}));\n'
                 validation_contents += f'}}\n'
         elif option['type'] in ['file-path', 'dir-path']:
             validation_contents += f'if (files::exists(s.{camel_name}))\n'
             validation_contents += f'{{\n'
             if option['type'] == 'file-path':
                 validation_contents += f'if (files::isDirectory(s.{camel_name}))\n'
                 validation_contents += f'{{\n'
-                validation_contents += f'    return Unexpected(formatError("{option["name"]} option: path should be a regular file: {{}}", s.{camel_name}));\n'
+                validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: path should be a regular file: {{}}", s.{camel_name}));\n'
                 validation_contents += f'}}\n'
             if option['type'] == 'dir-path':
                 validation_contents += f'if (!files::isDirectory(s.{camel_name}))\n'
                 validation_contents += f'{{\n'
-                validation_contents += f'    return Unexpected(formatError("{option["name"]} option: path should be a directory: {{}}", s.{camel_name}));\n'
+                validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: path should be a directory: {{}}", s.{camel_name}));\n'
                 validation_contents += f'}}\n'
             validation_contents += f'}}\n'
 
@@ -625,7 +625,7 @@ def option_validation_snippet(option):
                 validation_contents += f'// s.{camel_name} paths are required and have no default value\n'
                 validation_contents += f'if (s.{camel_name}.empty())\n'
                 validation_contents += f'{{\n'
-                validation_contents += f'    return Unexpected(formatError("{option["name"]} option is required"));'
+                validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option is required"));'
                 validation_contents += f'}}\n'
                 validation_contents += f'else\n'
                 validation_contents += f'{{\n'
@@ -694,7 +694,7 @@ def option_validation_snippet(option):
             if option['must-exist']:
                 validation_contents += f'    if (!files::exists(p))\n'
                 validation_contents += f'    {{\n'
-                validation_contents += f'        return Unexpected(formatError("{option["name"]} option: path does not exist: {{}}", p));\n'
+                validation_contents += f'        return Unexpected(formatError("`{option["name"]}` option: path does not exist: {{}}", p));\n'
                 validation_contents += f'    }}\n'
             if option['command-line-sink'] and 'filename-mapping' in option:
                 validation_contents += f'    auto f = files::getFileName(p);\n'
@@ -724,7 +724,7 @@ def option_validation_snippet(option):
             validation_contents += f'// s.{camel_name} is required with no default value.'
             validation_contents += f'if (s.{camel_name}.empty())\n'
             validation_contents += f'{{\n'
-            validation_contents += f'    return Unexpected(formatError("{option["name"]} option is required"));'
+            validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option is required"));'
             validation_contents += f'}}\n'
         else:
             validation_contents += f'// s.{camel_name} is not required and has no default value\n'
@@ -745,7 +745,7 @@ def option_validation_snippet(option):
             validation_contents += f'// s.{camel_name} is required with no default value.'
             validation_contents += f'if (s.{camel_name}.empty())\n'
             validation_contents += f'{{\n'
-            validation_contents += f'    return Unexpected(formatError("{option["name"]} option is required"));'
+            validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option is required"));'
             validation_contents += f'}}\n'
         else:
             validation_contents += f'// s.{camel_name} is not required and has no default value\n'
@@ -766,12 +766,12 @@ def option_validation_snippet(option):
         if 'min-value' in option:
             validation_contents += f'if (std::cmp_less(s.{camel_name}, {option["min-value"]}))\n'
             validation_contents += f'{{\n'
-            validation_contents += f'    return Unexpected(formatError("{option["name"]} option: value is less than {option["min-value"]}: {{}}", s.{camel_name}));\n'
+            validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: value is less than {option["min-value"]}: {{}}", s.{camel_name}));\n'
             validation_contents += f'}}\n'
         if 'max-value' in option:
             validation_contents += f'if (std::cmp_greater(s.{camel_name}, {option["max-value"]}))\n'
             validation_contents += f'{{\n'
-            validation_contents += f'    return Unexpected(formatError("{option["name"]} option: value is greater than {option["max-value"]}: {{}}", s.{camel_name}));\n'
+            validation_contents += f'    return Unexpected(formatError("`{option["name"]}` option: value is greater than {option["max-value"]}: {{}}", s.{camel_name}));\n'
             validation_contents += f'}}\n'
 
     contents += validation_contents
@@ -830,7 +830,7 @@ def generate_public_settings_cpp(config):
         for option in category['options']:
             if not option["command-line-only"]:
                 contents += f'        // {option["brief"]}\n'
-                contents += f'        io.mapOptional("{option["name"]}", s.{to_camel_case(option["name"])});\n'
+                contents += f'        io.mapOptional("`{option["name"]}`", s.{to_camel_case(option["name"])});\n'
     contents += '    }\n'
     contents += '};\n\n'
 
@@ -1060,7 +1060,7 @@ def generate_public_toolargs_cpp(config):
                 option_contents += f'    }}\n'
             option_contents += f'    else\n'
             option_contents += f'    {{\n'
-            option_contents += f'        return Unexpected(formatError("{option["name"]} option: invalid value: {{}}", this->{camel_name}));\n'
+            option_contents += f'        return Unexpected(formatError("`{option["name"]}` option: invalid value: {{}}", this->{camel_name}));\n'
             option_contents += f'    }}\n'
         else:
             option_contents += f'    s.{camel_name} = this->{camel_name};\n'
@@ -1149,7 +1149,7 @@ def to_cpp_default_value(option):
         if option_default.startswith('<'):
             closing_bracket = option_default.find('>')
             if closing_bracket == -1:
-                raise ValueError(f'Invalid default value {option_default} for option {option["name"]}')
+                raise ValueError(f'Invalid default value {option_default} for option `{option["name"]}`')
             reference_path = option_default[1:closing_bracket]
             if reference_path == 'config-dir':
                 option_default = '.' + option_default[closing_bracket + 1:]