From 82759d3899745efbacc10e5bf9737cc5b23097dd Mon Sep 17 00:00:00 2001 From: Kris Austin Date: Tue, 4 Aug 2026 09:01:45 -0500 Subject: [PATCH] fix: make the error dialog caret point at the character it's blaming (#14886) * fix: make the error dialog caret point at the character it's blaming Custom G-code parse errors print the offending line with a '^' under the character that broke, positioned with spaces so it only lines up in a fixed-width font. Since v2.3.2 these dialogs rendered entirely in the proportional UI font, so the caret drifted left of its column and landed on unrelated text. Render only the code excerpts (the offending source line and its caret) in the fixed-width face, leaving the surrounding prose in the UI font, and reserve the horizontal scrollbar's height so a long line does not clip. Rename the flag to has_code_excerpts to match what it now means. Fixes #14869 * refactor(GUI): use instead of for error excerpts wxHTML maps , , and to the same fixed-width handler, so this renders identically. is the non-deprecated tag and matches what the original code used. * fix(GUI): align the error caret with real spaces, not   The caret line was padded with   so its spaces would survive inline HTML. wxHTML measures every glyph by its font extent, so where the fixed font lacks a U+00A0 glyph the fallback renders it about twice as wide, and the all-  caret line outran the source, drifting the ^ to the right. Wrap the excerpts in a small tag, registered on the dialog's own parser, that switches on wxHTML literal-whitespace mode so the caret uses real spaces that match the source column in any font. It sits inside for the fixed face; 
 would do both but forces a blank line above it.

---------

Co-authored-by: Noisyfox 
---
 src/libslic3r/PlaceholderParser.cpp |   2 +
 src/slic3r/GUI/GUI.cpp              |   8 +-
 src/slic3r/GUI/GUI.hpp              |  10 +--
 src/slic3r/GUI/MsgDialog.cpp        | 124 ++++++++++++++++++++++++----
 src/slic3r/GUI/MsgDialog.hpp        |   6 +-
 5 files changed, 120 insertions(+), 30 deletions(-)

diff --git a/src/libslic3r/PlaceholderParser.cpp b/src/libslic3r/PlaceholderParser.cpp
index e3a4037590..3e29e0c172 100644
--- a/src/libslic3r/PlaceholderParser.cpp
+++ b/src/libslic3r/PlaceholderParser.cpp
@@ -1791,6 +1791,8 @@ namespace client
             // from UTF8 to UTF16 don't bail out.
             msg += boost::nowide::narrow(boost::nowide::widen(error_line));
             msg += '\n';
+            // The error dialog (MsgDialog.cpp) renders this excerpt monospaced. It recognizes a source
+            // line directly above a caret line of spaces and a single '^'.
             for (size_t i = 0; i < error_pos; ++ i)
                 msg += ' ';
             msg += "^\n";
diff --git a/src/slic3r/GUI/GUI.cpp b/src/slic3r/GUI/GUI.cpp
index 554ecd4a4e..29f8fc9749 100644
--- a/src/slic3r/GUI/GUI.cpp
+++ b/src/slic3r/GUI/GUI.cpp
@@ -256,18 +256,18 @@ void change_opt_value(DynamicPrintConfig& config, const t_config_option_key& opt
 	}
 }
 
-void show_error(wxWindow* parent, const wxString& message, bool monospaced_font)
+void show_error(wxWindow* parent, const wxString& message, bool has_code_excerpts)
 {
     wxGetApp().CallAfter([=] {
-        ErrorDialog msg(parent, message, monospaced_font);
+        ErrorDialog msg(parent, message, has_code_excerpts);
         msg.ShowModal();
     });
 }
 
-void show_error(wxWindow* parent, const char* message, bool monospaced_font)
+void show_error(wxWindow* parent, const char* message, bool has_code_excerpts)
 {
 	assert(message);
-	show_error(parent, wxString::FromUTF8(message), monospaced_font);
+	show_error(parent, wxString::FromUTF8(message), has_code_excerpts);
 }
 
 void show_error_id(int id, const std::string& message)
diff --git a/src/slic3r/GUI/GUI.hpp b/src/slic3r/GUI/GUI.hpp
index 357fd20a97..db882b79cf 100644
--- a/src/slic3r/GUI/GUI.hpp
+++ b/src/slic3r/GUI/GUI.hpp
@@ -40,11 +40,11 @@ extern void add_menus(wxMenuBar *menu, int event_preferences_changed, int event_
 // Change option value in config
 void change_opt_value(DynamicPrintConfig& config, const t_config_option_key& opt_key, const boost::any& value, int opt_index = 0);
 
-// If monospaced_font is true, the error message is displayed using html 
 tags,
-// so that the code formatting will be preserved. This is useful for reporting errors from the placeholder parser.
-void show_error(wxWindow* parent, const wxString& message, bool monospaced_font = false);
-void show_error(wxWindow* parent, const char* message, bool monospaced_font = false);
-inline void show_error(wxWindow* parent, const std::string& message, bool monospaced_font = false) { show_error(parent, message.c_str(), monospaced_font); }
+// If has_code_excerpts is true, code excerpts (a source line and the caret line below it) render
+// monospaced so the caret aligns. Used for placeholder-parser errors.
+void show_error(wxWindow* parent, const wxString& message, bool has_code_excerpts = false);
+void show_error(wxWindow* parent, const char* message, bool has_code_excerpts = false);
+inline void show_error(wxWindow* parent, const std::string& message, bool has_code_excerpts = false) { show_error(parent, message.c_str(), has_code_excerpts); }
 void show_error_id(int id, const std::string& message);   // For Perl
 void show_info(wxWindow* parent, const wxString& message, const wxString& title = wxString());
 void show_info(wxWindow* parent, const char* message, const char* title = nullptr);
diff --git a/src/slic3r/GUI/MsgDialog.cpp b/src/slic3r/GUI/MsgDialog.cpp
index ebf4db6846..7f1effd162 100644
--- a/src/slic3r/GUI/MsgDialog.cpp
+++ b/src/slic3r/GUI/MsgDialog.cpp
@@ -9,8 +9,13 @@
 #include 
 #include 
 #include 
+#include 
+
+#include 
 
 #include 
+#include 
+#include 
 
 #include "libslic3r/libslic3r.h"
 #include "libslic3r/Utils.hpp"
@@ -229,12 +234,82 @@ void MsgDialog::finalize()
 }
 
 
+// A placeholder-parser caret line, pointing at the column where parsing failed.
+static bool is_caret_line(const std::string &line)
+{
+    return std::count(line.begin(), line.end(), '^') == 1 &&
+           std::all_of(line.begin(), line.end(), [](char c) { return c == ' ' || c == '^'; });
+}
+
+// Tag each line as a code excerpt (a caret line or the source line above one) that must stay
+// monospaced for the '^' to align.
+static std::vector> classify_code_lines(const std::string &msg)
+{
+    std::vector lines;
+    boost::split(lines, msg, boost::is_any_of("\n"));
+    for (std::string &line : lines)
+        if (!line.empty() && line.back() == '\r')
+            line.pop_back();
+
+    std::vector> tagged;
+    tagged.reserve(lines.size());
+    for (size_t i = 0; i < lines.size(); ++i) {
+        bool is_code = is_caret_line(lines[i]) || (i + 1 < lines.size() && is_caret_line(lines[i + 1]));
+        tagged.emplace_back(std::move(lines[i]), is_code);
+    }
+    return tagged;
+}
+
+// Keeps whitespace literal so the caret's leading spaces survive.
+// Used inside , which supplies the fixed face. 
 does both but adds a blank line above it.
+class CodeExcerptTagHandler : public wxHtmlWinTagHandler
+{
+public:
+    wxString GetSupportedTags() override { return wxT("EXCERPT"); }
+    bool     HandleTag(const wxHtmlTag &tag) override
+    {
+        const wxHtmlWinParser::WhitespaceMode ws = m_WParser->GetWhitespaceMode();
+        m_WParser->SetWhitespaceMode(wxHtmlWinParser::Whitespace_Pre);
+        ParseInner(tag);
+        m_WParser->SetWhitespaceMode(ws);
+        return true;
+    }
+};
+
+// Render the message as HTML, monospacing only the code excerpts.
+static std::string format_parser_error_html(const std::string &msg)
+{
+    std::string out;
+    for (const auto &[text, is_code] : classify_code_lines(msg)) {
+        if (!out.empty()) out += "
"; // join, not trail; a trailing 
 forces a scrollbar
+        std::string escaped = xml_escape(text);
+        if (is_code)
+            out += "" + escaped + "";
+        else
+            out += escaped;
+    }
+    return out;
+}
+
+// Measure each line in the font it will render in, so the dialog fits the longest line without slack.
+static wxSize measure_mixed_text(wxWindow *parent, const std::string &msg, const wxFont &prose_font, const wxFont &code_font)
+{
+    wxClientDC dc(parent);
+    int width = 0, height = 0;
+    for (const auto &[text, is_code] : classify_code_lines(msg)) {
+        dc.SetFont(is_code ? code_font : prose_font);
+        width   = std::max(width, dc.GetTextExtent(wxString::FromUTF8(text.c_str())).GetWidth());
+        height += dc.GetCharHeight();
+    }
+    return wxSize(width, height);
+}
+
 // Text shown as HTML, so that mouse selection and Ctrl-V to copy will work.
 static void add_msg_content(wxWindow   *parent,
                             wxBoxSizer *content_sizer,
                             wxString    msg,
-                            bool        monospaced_font = false,
-                            bool        is_marked_msg   = false,
+                            bool        has_code_excerpts = false,
+                            bool        is_marked_msg     = false,
                             const wxString &link_text = "",
                             std::function link_callback = nullptr)
 {
@@ -243,7 +318,7 @@ static void add_msg_content(wxWindow   *parent,
 
     // count lines in the message
     int msg_lines = 0;
-    if (!monospaced_font) {
+    if (!has_code_excerpts) {
         int line_len = 55;// count of symbols in one line
         int start_line = 0;
         for (auto i = msg.begin(); i != msg.end(); ++i) {
@@ -300,13 +375,23 @@ static void add_msg_content(wxWindow   *parent,
         page_size       = wxSize(info_width, page_height);
     }
     else {
-        wxClientDC dc(parent);
-        dc.SetFont(font); // ORCA without this it calculates bigger size
-        wxSize msg_sz = dc.GetMultiLineTextExtent(msg) + parent->FromDIP(wxSize(10,5)); // added extra spacing to prevent wrapping
+        wxSize msg_sz;
+        if (has_code_excerpts) {
+            msg_sz = measure_mixed_text(parent, msg.ToUTF8().data(), font, monospace);
+        } else {
+            wxClientDC dc(parent);
+            dc.SetFont(font); // ORCA without this it calculates bigger size
+            msg_sz = dc.GetMultiLineTextExtent(msg);
+        }
+        msg_sz += parent->FromDIP(wxSize(10,5)); // added extra spacing to prevent wrapping
 
-        page_size = wxSize(std::min(msg_sz.GetX(), info_width), std::min(msg_sz.GetY(), info_width));
+        int page_height = msg_sz.GetY();
+        // Reserve the horizontal scrollbar's height, or it clips the last line.
+        if (msg_sz.GetX() > info_width)
+            page_height += wxSystemSettings::GetMetric(wxSYS_HSCROLL_Y, parent);
+        page_size = wxSize(std::min(msg_sz.GetX(), info_width), std::min(page_height, info_width));
         // Extra line breaks in message dialog
-        if (link_text.IsEmpty() && !link_callback && is_marked_msg == false) {//for common text
+        if (link_text.IsEmpty() && !link_callback && is_marked_msg == false && !has_code_excerpts) {//for common text
             html->Destroy();
             if (msg_sz.GetX() < info_width) {//No need for line breaks
                 info_width = msg_sz.GetX();
@@ -337,12 +422,15 @@ static void add_msg_content(wxWindow   *parent,
     }
     html->SetMinSize(page_size);
 
-    std::string msg_escaped = xml_escape(msg.ToUTF8().data(), is_marked_msg);
-    boost::replace_all(msg_escaped, "\r\n", "
");
-    boost::replace_all(msg_escaped, "\n", "
");
-    if (monospaced_font)
-        // Code formatting will be preserved. This is useful for reporting errors from the placeholder parser.
-        msg_escaped = std::string("
") + msg_escaped + "
";
+    std::string msg_escaped;
+    if (has_code_excerpts) {
+        html->GetParser()->AddTagHandler(new CodeExcerptTagHandler());
+        msg_escaped = format_parser_error_html(msg.ToUTF8().data());
+    } else {
+        msg_escaped = xml_escape(msg.ToUTF8().data(), is_marked_msg);
+        boost::replace_all(msg_escaped, "\r\n", "
");
+        boost::replace_all(msg_escaped, "\n", "
");
+    }
 
     if (!link_text.IsEmpty() && link_callback) {
         msg_escaped += "" + std::string(link_text.ToUTF8().data()) + "";
@@ -360,15 +448,15 @@ static void add_msg_content(wxWindow   *parent,
 
 // ErrorDialog
 
-ErrorDialog::ErrorDialog(wxWindow *parent, const wxString &temp_msg, bool monospaced_font)
+ErrorDialog::ErrorDialog(wxWindow *parent, const wxString &temp_msg, bool has_code_excerpts)
     : MsgDialog(parent, wxString::Format(_(L("%s error")), SLIC3R_APP_FULL_NAME),
                         wxString::Format(_(L("%s has encountered an error")), SLIC3R_APP_FULL_NAME), wxOK)
     , msg(temp_msg)
 {
-    add_msg_content(this, content_sizer, msg, monospaced_font);
+    add_msg_content(this, content_sizer, msg, has_code_excerpts);
 
-	// Use a small bitmap with monospaced font, as the error text will not be wrapped.
-	logo->SetBitmap(create_scaled_bitmap("OrcaSlicer_192px_grayscale.png", this, monospaced_font ? 48 : /*1*/64));
+	// Use a small bitmap for code excerpts, which cannot wrap and so need the width.
+	logo->SetBitmap(create_scaled_bitmap("OrcaSlicer_192px_grayscale.png", this, has_code_excerpts ? 48 : /*1*/64));
 
     SetMaxSize(MSG_DLG_MAX_SIZE);
 
diff --git a/src/slic3r/GUI/MsgDialog.hpp b/src/slic3r/GUI/MsgDialog.hpp
index 174d734336..90fd160310 100644
--- a/src/slic3r/GUI/MsgDialog.hpp
+++ b/src/slic3r/GUI/MsgDialog.hpp
@@ -106,9 +106,9 @@ protected:
 class ErrorDialog : public MsgDialog
 {
 public:
-	// If monospaced_font is true, the error message is displayed using html 
 tags,
-	// so that the code formatting will be preserved. This is useful for reporting errors from the placeholder parser.
-	ErrorDialog(wxWindow *parent, const wxString &temp_msg, bool courier_font);
+	// If has_code_excerpts is true, code excerpts (a source line and the caret line below it) render
+	// monospaced so the caret aligns. Used for placeholder-parser errors.
+	ErrorDialog(wxWindow *parent, const wxString &temp_msg, bool has_code_excerpts);
 	ErrorDialog(ErrorDialog &&) = delete;
 	ErrorDialog(const ErrorDialog &) = delete;
 	ErrorDialog &operator=(ErrorDialog &&) = delete;