From 8c5037a48bd08cf0d864fd5832576f0c3481c588 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 21 May 2018 15:28:26 -0700 Subject: [PATCH] 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 | 91 +++++++++++-------- common/dialogs/wx_html_report_panel.h | 22 +++-- common/reporter.cpp | 18 ++++ eeschema/annotate.cpp | 4 +- eeschema/dialogs/dialog_annotate.cpp | 2 +- eeschema/dialogs/dialog_erc.cpp | 4 +- eeschema/dialogs/dialog_symbol_remap.cpp | 4 +- eeschema/netlist_generator.cpp | 8 +- gerbview/job_file_reader.cpp | 2 +- include/reporter.h | 51 ++++++++++- pcbnew/board_netlist_updater.cpp | 10 +- pcbnew/dialogs/dialog_exchange_footprints.cpp | 4 +- pcbnew/dialogs/dialog_export_step.cpp | 8 +- pcbnew/dialogs/dialog_netlist.cpp | 6 +- pcbnew/dialogs/dialog_update_pcb.cpp | 6 +- 15 files changed, 161 insertions(+), 79 deletions(-) diff --git a/common/dialogs/wx_html_report_panel.cpp b/common/dialogs/wx_html_report_panel.cpp index f8112fdeb5..ce7279d95f 100644 --- a/common/dialogs/wx_html_report_panel.cpp +++ b/common/dialogs/wx_html_report_panel.cpp @@ -19,6 +19,8 @@ * with this program. If not, see . */ +#include + #include "wx_html_report_panel.h" #include @@ -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(); } diff --git a/common/dialogs/wx_html_report_panel.h b/common/dialogs/wx_html_report_panel.h index f83f33d548..0d4221defa 100644 --- a/common/dialogs/wx_html_report_panel.h +++ b/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; }; diff --git a/common/reporter.cpp b/common/reporter.cpp index 5e971f9aa3..b9dce9e91f 100644 --- a/common/reporter.cpp +++ b/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; diff --git a/eeschema/annotate.cpp b/eeschema/annotate.cpp index d083284417..e690d821fa 100644 --- a/eeschema/annotate.cpp +++ b/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(); diff --git a/eeschema/dialogs/dialog_annotate.cpp b/eeschema/dialogs/dialog_annotate.cpp index d45a049919..76b120be9c 100644 --- a/eeschema/dialogs/dialog_annotate.cpp +++ b/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(); diff --git a/eeschema/dialogs/dialog_erc.cpp b/eeschema/dialogs/dialog_erc.cpp index 256ca2bbf0..5b7c7a3a30 100644 --- a/eeschema/dialogs/dialog_erc.cpp +++ b/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 ) { diff --git a/eeschema/dialogs/dialog_symbol_remap.cpp b/eeschema/dialogs/dialog_symbol_remap.cpp index f81685fdec..3eda4bab72 100644 --- a/eeschema/dialogs/dialog_symbol_remap.cpp +++ b/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 ); } } } diff --git a/eeschema/netlist_generator.cpp b/eeschema/netlist_generator.cpp index 7b5a20a94f..d8757aa979 100644 --- a/eeschema/netlist_generator.cpp +++ b/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() ) { diff --git a/gerbview/job_file_reader.cpp b/gerbview/job_file_reader.cpp index 8cc29098e1..ec617b983f 100644 --- a/gerbview/job_file_reader.cpp +++ b/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; diff --git a/include/reporter.h b/include/reporter.h index 116069b790..15789d3378 100644 --- a/include/reporter.h +++ b/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; }; diff --git a/pcbnew/board_netlist_updater.cpp b/pcbnew/board_netlist_updater.cpp index d6a42eb6e8..0066e2771c 100644 --- a/pcbnew/board_netlist_updater.cpp +++ b/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; diff --git a/pcbnew/dialogs/dialog_exchange_footprints.cpp b/pcbnew/dialogs/dialog_exchange_footprints.cpp index 2f6464faf5..4aee9e4108 100644 --- a/pcbnew/dialogs/dialog_exchange_footprints.cpp +++ b/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(); diff --git a/pcbnew/dialogs/dialog_export_step.cpp b/pcbnew/dialogs/dialog_export_step.cpp index 3a8e567a6d..492d4c9b16 100644 --- a/pcbnew/dialogs/dialog_export_step.cpp +++ b/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 ); } } diff --git a/pcbnew/dialogs/dialog_netlist.cpp b/pcbnew/dialogs/dialog_netlist.cpp index dbea22d3c3..cba95c9bc0 100644 --- a/pcbnew/dialogs/dialog_netlist.cpp +++ b/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 ); } diff --git a/pcbnew/dialogs/dialog_update_pcb.cpp b/pcbnew/dialogs/dialog_update_pcb.cpp index c20da7c2a8..719161b37f 100644 --- a/pcbnew/dialogs/dialog_update_pcb.cpp +++ b/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;