Optionally sort reporter messages

It can be useful to have similar class messages grouped together so that
error messages in a larger report don't get lost among the
warning/info/action messages.

This patch allows sorting of messages for dialogs that benefit from
organization.  Default behavior of reporter messages remains
unchanged by this patch.

Fixes: lp:1772090
* https://bugs.launchpad.net/kicad/+bug/1772090

common/dialogs/wx_html_report_panel.cpp

@@ -19,6 +19,8 @@
  * with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <algorithm>
+
 #include "wx_html_report_panel.h"
 
 #include <wildcards_and_files_ext.h>
@@ -62,15 +64,19 @@ REPORTER& WX_HTML_REPORT_PANEL::Reporter()
 }
 
 
-void WX_HTML_REPORT_PANEL::Report( const wxString& aText, REPORTER::SEVERITY aSeverity )
+void WX_HTML_REPORT_PANEL::Report( const wxString& aText, REPORTER::SEVERITY aSeverity,
+        REPORTER::LOCATION aLocation )
 {
     REPORT_LINE line;
     line.message = aText;
     line.severity = aSeverity;
 
-    m_report.push_back( line );
-
-    m_html += generateHtml( line );
+    if( aLocation == REPORTER::LOC_HEAD )
+        m_reportHead.push_back( line );
+    else if( aLocation == REPORTER::LOC_TAIL )
+        m_reportTail.push_back( line );
+    else
+        m_report.push_back( line );
 
     if( !m_lazyUpdate )
     {
@@ -86,9 +92,29 @@ void WX_HTML_REPORT_PANEL::SetLazyUpdate( bool aLazyUpdate )
 }
 
 
-void WX_HTML_REPORT_PANEL::Flush()
+void WX_HTML_REPORT_PANEL::Flush( bool aSort )
 {
-    m_htmlView->SetPage( addHeader( m_html ) );
+    wxString html;
+
+    if( aSort )
+    {
+        std::sort( m_report.begin(), m_report.end(),
+                []( REPORT_LINE& a, REPORT_LINE& b)
+                {
+                    return a.severity < b.severity;
+                });
+    }
+
+    for( auto line : m_reportHead )
+        html += generateHtml( line );
+
+    for( auto line : m_report )
+        html += generateHtml( line );
+
+    for( auto line : m_reportTail )
+        html += generateHtml( line );
+
+    m_htmlView->SetPage( addHeader( html ) );
     scrollToBottom();
 }
 
@@ -192,20 +218,6 @@ void WX_HTML_REPORT_PANEL::updateBadges()
 }
 
 
-void WX_HTML_REPORT_PANEL::refreshView()
-{
-    wxString html;
-
-    for( const REPORT_LINE& l : m_report )
-    {
-        html += generateHtml( l );
-    }
-
-    m_htmlView->SetPage( addHeader( html ) );
-    scrollToBottom();
-}
-
-
 wxString WX_HTML_REPORT_PANEL::addHeader( const wxString& aBody )
 {
     wxColour bgcolor = wxSystemSettings::GetColour( wxSYS_COLOUR_WINDOW );
@@ -310,7 +322,7 @@ void WX_HTML_REPORT_PANEL::onCheckBoxShowAll( wxCommandEvent& event )
         m_showAll = false;
 
     syncCheckboxes();
-    refreshView();
+    Flush( true );
 }
 
 
@@ -326,45 +338,45 @@ void WX_HTML_REPORT_PANEL::syncCheckboxes()
 
 void WX_HTML_REPORT_PANEL::onCheckBoxShowWarnings( wxCommandEvent& event )
 {
-    if ( event.IsChecked() )
+    if( event.IsChecked() )
         m_severities |= REPORTER::RPT_WARNING;
     else
         m_severities &= ~REPORTER::RPT_WARNING;
 
-     refreshView();
+    Flush( true );
 }
 
 
 void WX_HTML_REPORT_PANEL::onCheckBoxShowErrors( wxCommandEvent& event )
 {
-    if ( event.IsChecked() )
-         m_severities |= REPORTER::RPT_ERROR;
-     else
-         m_severities &= ~REPORTER::RPT_ERROR;
+    if( event.IsChecked() )
+        m_severities |= REPORTER::RPT_ERROR;
+    else
+        m_severities &= ~REPORTER::RPT_ERROR;
 
-     refreshView();
+    Flush( true );
 }
 
 
 void WX_HTML_REPORT_PANEL::onCheckBoxShowInfos( wxCommandEvent& event )
 {
-    if ( event.IsChecked() )
-         m_severities |= REPORTER::RPT_INFO;
-     else
-         m_severities &= ~REPORTER::RPT_INFO;
+    if( event.IsChecked() )
+        m_severities |= REPORTER::RPT_INFO;
+    else
+        m_severities &= ~REPORTER::RPT_INFO;
 
-     refreshView();
+    Flush( true );
 }
 
 
 void WX_HTML_REPORT_PANEL::onCheckBoxShowActions( wxCommandEvent& event )
 {
-    if ( event.IsChecked() )
-         m_severities |= REPORTER::RPT_ACTION;
-     else
-         m_severities &= ~REPORTER::RPT_ACTION;
+    if( event.IsChecked() )
+        m_severities |= REPORTER::RPT_ACTION;
+    else
+        m_severities &= ~REPORTER::RPT_ACTION;
 
-     refreshView();
+    Flush( true );
 }
 
 
@@ -406,8 +418,9 @@ void WX_HTML_REPORT_PANEL::onBtnSaveToFile( wxCommandEvent& event )
 
 void WX_HTML_REPORT_PANEL::Clear()
 {
-    m_html.clear();
     m_report.clear();
+    m_reportHead.clear();
+    m_reportTail.clear();
 }
 
 

common/dialogs/wx_html_report_panel.h

@@ -52,8 +52,14 @@ public:
     ///> returns the reporter object that reports to this panel
     REPORTER& Reporter();
 
-    ///> reports a string directly.
-    void Report( const wxString& aText, REPORTER::SEVERITY aSeverity );
+    /**
+     * Reports the string
+     * @param aText string message to report
+     * @param aSeverity string classification level bitfield
+     * @param aLocation REPORTER::LOCATION enum for placement of message
+     */
+    void Report( const wxString& aText, REPORTER::SEVERITY aSeverity,
+            REPORTER::LOCATION aLocation = REPORTER::LOC_BODY );
 
     ///> clears the report panel
     void Clear();
@@ -70,7 +76,8 @@ public:
     void SetLazyUpdate( bool aLazyUpdate );
 
     ///> Forces updating the HTML page, after the report is built in lazy mode
-    void Flush();
+    ///> If aSort = true, the body messages will be ordered by severity
+    void Flush( bool aSort = false );
 
     ///> Set the visible severity filter.
     ///> if aSeverities < 0 the m_showAll option is set
@@ -94,7 +101,6 @@ private:
     wxString generatePlainText( const REPORT_LINE& aLine );
     void updateBadges();
 
-    void refreshView();
     void scrollToBottom();
     void syncCheckboxes();
 
@@ -111,6 +117,12 @@ private:
     ///> copy of the report, stored for filtering
     REPORT_LINES m_report;
 
+    ///> Lines to print at the very end of the report, regardless of sorting
+    REPORT_LINES m_reportTail;
+
+    ///> Lines to print at the very beginning of the report, regardless of sorting
+    REPORT_LINES m_reportHead;
+
     ///> the reporter
     WX_HTML_PANEL_REPORTER m_reporter;
 
@@ -120,8 +132,6 @@ private:
     ///> show all messages flag (overrides m_severities)
     bool m_showAll;
 
-    wxString m_html;
-
     bool m_lazyUpdate;
 };
 

common/reporter.cpp

@@ -73,6 +73,24 @@ REPORTER& WX_HTML_PANEL_REPORTER::Report( const wxString& aText, SEVERITY aSever
     return *this;
 }
 
+REPORTER& WX_HTML_PANEL_REPORTER::ReportTail( const wxString& aText, SEVERITY aSeverity )
+{
+    wxCHECK_MSG( m_panel != NULL, *this,
+                 wxT( "No WX_HTML_REPORT_PANEL object defined in WX_HTML_PANEL_REPORTER." ) );
+
+    m_panel->Report( aText, aSeverity, LOC_TAIL );
+    return *this;
+}
+
+REPORTER& WX_HTML_PANEL_REPORTER::ReportHead( const wxString& aText, SEVERITY aSeverity )
+{
+    wxCHECK_MSG( m_panel != NULL, *this,
+                 wxT( "No WX_HTML_REPORT_PANEL object defined in WX_HTML_PANEL_REPORTER." ) );
+
+    m_panel->Report( aText, aSeverity, LOC_HEAD );
+    return *this;
+}
+
 bool WX_HTML_PANEL_REPORTER::HasMessage() const
 {
     return m_panel->Count( REPORTER::RPT_ERROR | REPORTER::RPT_WARNING ) > 0;

eeschema/annotate.cpp

@@ -110,7 +110,7 @@ void SCH_EDIT_FRAME::AnnotateComponents( bool              aAnnotateSchematic,
         {
             wxString msg;
             msg.Printf( _( "%d duplicate time stamps were found and replaced." ), count );
-            aReporter.Report( msg, REPORTER::RPT_WARNING );
+            aReporter.ReportTail( msg, REPORTER::RPT_WARNING );
         }
     }
 
@@ -228,7 +228,7 @@ void SCH_EDIT_FRAME::AnnotateComponents( bool              aAnnotateSchematic,
 
     // Final control (just in case ... ).
     if( !CheckAnnotate( aReporter, !aAnnotateSchematic ) )
-        aReporter.Report( _( "Annotation complete." ), REPORTER::RPT_ACTION );
+        aReporter.ReportTail( _( "Annotation complete." ), REPORTER::RPT_ACTION );
 
     OnModify();
 

eeschema/dialogs/dialog_annotate.cpp

@@ -223,7 +223,7 @@ void DIALOG_ANNOTATE::OnApplyClick( wxCommandEvent& event )
                                   (ANNOTATE_OPTION_T) GetAnnotateAlgo(), GetStartNumber(),
                                   GetResetItems() , true, GetLockUnits(), reporter );
 
-    m_MessageWindow->Flush();                   // Now update to show all messages
+    m_MessageWindow->Flush( true );                   // Now update to show all messages
 
     m_Parent->GetCanvas()->Refresh();
 

eeschema/dialogs/dialog_erc.cpp

@@ -475,7 +475,7 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter )
     if( m_parent->CheckAnnotate( aReporter, false ) )
     {
         if( aReporter.HasMessage() )
-            aReporter.Report( _( "Annotation required!" ), REPORTER::RPT_ERROR );
+            aReporter.ReportTail( _( "Annotation required!" ), REPORTER::RPT_ERROR );
 
         return;
     }
@@ -624,7 +624,7 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter )
     m_parent->GetCanvas()->Refresh();
 
     // Display message
-    aReporter.Report( _( "Finished" ), REPORTER::RPT_INFO );
+    aReporter.ReportTail( _( "Finished" ), REPORTER::RPT_INFO );
 
     if( m_writeErcFile )
     {

eeschema/dialogs/dialog_symbol_remap.cpp

@@ -214,10 +214,10 @@ void DIALOG_SYMBOL_REMAP::createProjectSymbolLibTable( REPORTER& aReporter )
             {
                 msg.Printf( _( "Failed to write project symbol library table. Error:\n  %s" ),
                             ioe.What() );
-                aReporter.Report( msg, REPORTER::RPT_ERROR );
+                aReporter.ReportTail( msg, REPORTER::RPT_ERROR );
             }
 
-            aReporter.Report( _( "Created project symbol library table.\n" ), REPORTER::RPT_INFO );
+            aReporter.ReportTail( _( "Created project symbol library table.\n" ), REPORTER::RPT_INFO );
         }
     }
 }

eeschema/netlist_generator.cpp

@@ -114,16 +114,14 @@ bool SCH_EDIT_FRAME::WriteNetListFile( NETLIST_OBJECT_LIST* aConnectedItemsList,
 
             msg << _( "Run command:" ) << wxT( "\n" ) << commandLine << wxT( "\n\n" );
 
-            aReporter->Report( msg, REPORTER::RPT_ACTION );
+            aReporter->ReportHead( msg, REPORTER::RPT_ACTION );
 
             if( diag != 0 )
-                aReporter->Report( wxString::Format(
+                aReporter->ReportTail( wxString::Format(
                                     _("Command error. Return code %d" ), diag ),
                                     REPORTER::RPT_ERROR );
             else
-                aReporter->Report( _( "Success" ), REPORTER::RPT_INFO );
-
-            *aReporter << wxT("\n");
+                aReporter->ReportTail( _( "Success" ), REPORTER::RPT_INFO );
 
             if( output.GetCount() )
             {

gerbview/job_file_reader.cpp

@@ -145,7 +145,7 @@ bool GERBER_JOBFILE_READER::ReadGerberJobFile()
     else
     {
         if( m_reporter )
-            m_reporter->Report( _( "This job file uses an outdated format. Please, recreate it" ),
+            m_reporter->ReportTail( _( "This job file uses an outdated format. Please, recreate it" ),
                                 REPORTER::RPT_WARNING );
 
         return false;

include/reporter.h

@@ -61,13 +61,34 @@ class WX_HTML_REPORT_PANEL;
 class REPORTER {
 
 public:
-    ///> Severity of the reported messages.
+    /**
+     *  Severity of the reported messages.
+     *  Undefined are default status messages
+     *  Info are processing messages for which no action is taken
+     *  Action messages are items that modify the file(s) as expected
+     *  Warning messages are items that might be problematic but don't prevent
+     *    the process from completing
+     *  Error messages are items that prevent the process from completing
+     */
+    //
     enum SEVERITY {
         RPT_UNDEFINED = 0x0,
         RPT_INFO      = 0x1,
-        RPT_WARNING   = 0x2,
-        RPT_ERROR     = 0x4,
-        RPT_ACTION    = 0x8
+        RPT_ACTION    = 0x2,
+        RPT_WARNING   = 0x4,
+        RPT_ERROR     = 0x8
+    };
+
+    /**
+     * Location where the message is to be reported.
+     * LOC_HEAD messages are printed before all others (typically intro messages)
+     * LOC_BODY messages are printed in the middle
+     * LOC_TAIL messages are printed after all others (typically status messages)
+     */
+    enum LOCATION {
+        LOC_HEAD = 0,
+        LOC_BODY,
+        LOC_TAIL
     };
 
     /**
@@ -81,6 +102,24 @@ public:
 
     virtual REPORTER& Report( const wxString& aText, SEVERITY aSeverity = RPT_UNDEFINED ) = 0;
 
+    /**
+     * Function ReportTail
+     * Places the report at the end of the list, for objects that support report ordering
+     */
+    virtual REPORTER& ReportTail( const wxString& aText, SEVERITY aSeverity = RPT_UNDEFINED )
+    {
+        return Report( aText, aSeverity );
+    }
+
+    /**
+     * Function ReportHead
+     * Places the report at the beginning of the list for objects that support ordering
+     */
+    virtual REPORTER& ReportHead( const wxString& aText, SEVERITY aSeverity = RPT_UNDEFINED )
+    {
+        return Report( aText, aSeverity );
+    }
+
     REPORTER& Report( const char* aText, SEVERITY aSeverity = RPT_UNDEFINED );
 
     REPORTER& operator <<( const wxString& aText ) { return Report( aText ); }
@@ -155,6 +194,10 @@ public:
 
     REPORTER& Report( const wxString& aText, SEVERITY aSeverity = RPT_UNDEFINED ) override;
 
+    REPORTER& ReportTail( const wxString& aText, SEVERITY aSeverity = RPT_UNDEFINED ) override;
+
+    REPORTER& ReportHead( const wxString& aText, SEVERITY aSeverity = RPT_UNDEFINED ) override;
+
     bool HasMessage() const override;
 };
 

pcbnew/board_netlist_updater.cpp

@@ -774,15 +774,15 @@ bool BOARD_NETLIST_UPDATER::UpdateNetlist( NETLIST& aNetlist )
     }
 
     // Update the ratsnest
-    m_reporter->Report( wxT( "" ), REPORTER::RPT_ACTION );
-    m_reporter->Report( wxT( "" ), REPORTER::RPT_ACTION );
+    m_reporter->ReportTail( wxT( "" ), REPORTER::RPT_ACTION );
+    m_reporter->ReportTail( wxT( "" ), REPORTER::RPT_ACTION );
 
     msg.Printf( _( "Total warnings: %d, errors: %d." ), m_warningCount, m_errorCount );
-    m_reporter->Report( msg, REPORTER::RPT_ACTION );
+    m_reporter->ReportTail( msg, REPORTER::RPT_ACTION );
 
     if( m_errorCount )
     {
-        m_reporter->Report( _( "Errors occurred during the netlist update. Unless you "
+        m_reporter->ReportTail( _( "Errors occurred during the netlist update. Unless you "
                                "fix them, your board will not be consistent with the schematics." ),
                             REPORTER::RPT_ERROR );
 
@@ -790,7 +790,7 @@ bool BOARD_NETLIST_UPDATER::UpdateNetlist( NETLIST& aNetlist )
     }
     else
     {
-        m_reporter->Report( _( "Netlist update successful!" ), REPORTER::RPT_ACTION );
+        m_reporter->ReportTail( _( "Netlist update successful!" ), REPORTER::RPT_ACTION );
     }
 
     return true;

pcbnew/dialogs/dialog_exchange_footprints.cpp

@@ -310,7 +310,7 @@ void DIALOG_EXCHANGE_FOOTPRINTS::OnApplyClick( wxCommandEvent& event )
     bool result = false;
 
     m_MessageWindow->Clear();
-    m_MessageWindow->Flush();
+    m_MessageWindow->Flush( true );
 
     if( getMatchMode() == ID_MATCH_FP_REF && m_currentModule )
         result = changeCurrentFootprint();
@@ -334,7 +334,7 @@ void DIALOG_EXCHANGE_FOOTPRINTS::RebuildCmpList( wxCommandEvent& event )
     wxString    msg;
     REPORTER& reporter = m_MessageWindow->Reporter();
     m_MessageWindow->Clear();
-    m_MessageWindow->Flush();
+    m_MessageWindow->Flush( true );
 
     // Build the .cmp file name from the board name
     wxFileName fn = m_parent->GetBoard()->GetFileName();

pcbnew/dialogs/dialog_export_step.cpp

@@ -309,7 +309,7 @@ void DIALOG_EXPORT_STEP::onExportButton( wxCommandEvent& aEvent )
     bool success = false;
     wxArrayString output, errors;
     REPORTER& reporter = m_messagesPanel->Reporter();
-    reporter.Report( wxString::Format( _( "Executing '%s'" ), cmdK2S ), REPORTER::RPT_ACTION );
+    reporter.ReportHead( wxString::Format( _( "Executing '%s'" ), cmdK2S ), REPORTER::RPT_ACTION );
 
     {
         wxBusyCursor dummy;
@@ -334,17 +334,17 @@ void DIALOG_EXPORT_STEP::onExportButton( wxCommandEvent& aEvent )
     {
         if( !success )
         {
-            reporter.Report( _( "Unable to create STEP file.  Check that the board has a "
+            reporter.ReportTail( _( "Unable to create STEP file.  Check that the board has a "
                                         "valid outline and models." ), REPORTER::RPT_ERROR );
         }
         else
         {
-            reporter.Report( _( "STEP file has been created, but there are warnings." ),
+            reporter.ReportTail( _( "STEP file has been created, but there are warnings." ),
                     REPORTER::RPT_INFO );
         }
     }
     else
     {
-        reporter.Report( _( "STEP file has been created successfully." ), REPORTER::RPT_INFO );
+        reporter.ReportTail( _( "STEP file has been created successfully." ), REPORTER::RPT_INFO );
     }
 }

pcbnew/dialogs/dialog_netlist.cpp

@@ -188,14 +188,14 @@ void DIALOG_NETLIST::OnReadNetlistFileClick( wxCommandEvent& event )
 
     wxString msg;
     msg.Printf( _( "Reading netlist file \"%s\".\n" ), GetChars( netlistFileName ) );
-    reporter.Report( msg, REPORTER::RPT_INFO );
+    reporter.ReportHead( msg, REPORTER::RPT_INFO );
 
     if( m_Select_By_Timestamp->GetSelection() == 1 )
         msg = _( "Using time stamps to match components and footprints.\n" );
     else
         msg = _( "Using references to match components and footprints.\n" );
 
-    reporter.Report( msg, REPORTER::RPT_INFO );
+    reporter.ReportHead( msg, REPORTER::RPT_INFO );
     m_MessageWindow->SetLazyUpdate( true ); // use a "lazy" update to speed up the creation of the report
                                             // (The window is not updated for each message)
 
@@ -208,7 +208,7 @@ void DIALOG_NETLIST::OnReadNetlistFileClick( wxCommandEvent& event )
                               m_checkDryRun->GetValue() );
     // The creation of the report was made without window update:
     // the full page must be displayed
-    m_MessageWindow->Flush();
+    m_MessageWindow->Flush( true );
 }
 
 

pcbnew/dialogs/dialog_update_pcb.cpp

@@ -95,9 +95,9 @@ void DIALOG_UPDATE_PCB::PerformUpdate( bool aDryRun )
     {
         wxString msg;
 
-        reporter.Report( _( "Failed to load one or more footprints. Please add the missing libraries in PCBNew configuration. "
+        reporter.ReportTail( _( "Failed to load one or more footprints. Please add the missing libraries in PCBNew configuration. "
                             "The PCB will not update completely." ), REPORTER::RPT_ERROR );
-        reporter.Report( error.What(), REPORTER::RPT_INFO );
+        reporter.ReportTail( error.What(), REPORTER::RPT_INFO );
     }
 
     BOARD_NETLIST_UPDATER updater( m_frame, m_frame->GetBoard() );
@@ -109,7 +109,7 @@ void DIALOG_UPDATE_PCB::PerformUpdate( bool aDryRun )
     updater.SetDeleteSinglePadNets( false );
     updater.UpdateNetlist( *m_netlist );
 
-    m_messagePanel->Flush();
+    m_messagePanel->Flush( true );
 
     if( aDryRun )
         return;