From b9fd18d150009da529398cab9bf10e1243672364 Mon Sep 17 00:00:00 2001
From: Mikolaj Wielgus <wielgusmikolaj@gmail.com>
Date: Mon, 10 May 2021 01:32:49 +0200
Subject: [PATCH] Minor UI changes, display workbook in title and some fixes

---
 eeschema/sim/sim_plot_frame.cpp      | 124 +++++++++++++++++++++------
 eeschema/sim/sim_plot_frame.h        |  14 ++-
 eeschema/sim/sim_plot_frame_base.cpp |   6 +-
 eeschema/sim/sim_plot_frame_base.fbp |  14 +--
 eeschema/sim/sim_workbook.cpp        |  17 ++--
 eeschema/sim/sim_workbook.h          |  10 ++-
 eeschema/sim/spice_settings.cpp      |   4 +-
 eeschema/sim/spice_settings.h        |   6 +-
 8 files changed, 142 insertions(+), 53 deletions(-)

diff --git a/eeschema/sim/sim_plot_frame.cpp b/eeschema/sim/sim_plot_frame.cpp
index 968dded62a..d695912f90 100644
--- a/eeschema/sim/sim_plot_frame.cpp
+++ b/eeschema/sim/sim_plot_frame.cpp
@@ -306,21 +306,46 @@ void SIM_PLOT_FRAME::initWorkbook()
 {
     m_workbook = std::make_unique<SIM_WORKBOOK>();
 
-    if( m_simulator->Settings()->GetWorkbookPath().IsEmpty() )
+    if( !m_simulator->Settings()->GetWorkbookFilename().IsEmpty() )
     {
-        m_simulator->Settings()->SetWorkbookPath( Prj().GetProjectName() + ".wbk" );
-    }
-    else
-    {
-        wxFileName filename = m_simulator->Settings()->GetWorkbookPath();
+        wxFileName filename = m_simulator->Settings()->GetWorkbookFilename();
         filename.SetPath( Prj().GetProjectPath() );
 
         if( !loadWorkbook( filename.GetFullPath() ) )
-            DisplayErrorMessage( this, _( "There was an error while opening the workbook file" ) );
+        {
+            // Empty the workbook filename to prevent error messages from appearing again
+            m_simulator->Settings()->SetWorkbookFilename( "" );
+        }
     }
 }
 
 
+void SIM_PLOT_FRAME::updateTitle()
+{
+    wxFileName filename = m_simulator->Settings()->GetWorkbookFilename();
+
+    bool readOnly = false;
+    bool unsaved = false;
+
+    if( filename.IsOk() && filename.FileExists() )
+        readOnly = !filename.IsFileWritable();
+    else
+        unsaved = true;
+
+    SetTitle( wxString::Format( wxT( "%s%s %s%s\u2014 " ) + _( "Spice Simulator" ),
+                                m_workbook->IsModified() ? "*" : "",
+                                filename.GetName(),
+                                readOnly ? _( "[Read Only]" ) + wxS( " " ) : "",
+                                unsaved ? _( "[Unsaved]" ) + wxS( " " ) : "" ) );
+}
+
+
+void SIM_PLOT_FRAME::onModify()
+{
+    updateTitle();
+}
+
+
 // A small helper struct to handle bitmaps initialisation in menus
 struct BM_MENU_INIT_ITEM
 {
@@ -490,8 +515,10 @@ SIM_PANEL_BASE* SIM_PLOT_FRAME::NewPlotPanel( wxString aSimCommand )
     pageTitle.Prepend( wxString::Format( _( "Plot%u - " ), (unsigned int) ++m_plotNumber ) );
 
     m_workbook->AddPlotPanel( plotPanel );
+
     m_plotNotebook->AddPage( dynamic_cast<wxWindow*>( plotPanel ), pageTitle, true );
 
+    onModify();
     return plotPanel;
 }
 
@@ -658,6 +685,7 @@ void SIM_PLOT_FRAME::removePlot( const wxString& aPlotName, bool aErase )
     updateSignalList();
     wxCommandEvent dummy;
     onCursorUpdate( dummy );
+    onModify();
 }
 
 
@@ -766,6 +794,7 @@ bool SIM_PLOT_FRAME::updatePlot( const TRACE_DESC& aDescriptor, SIM_PLOT_PANEL*
                 offset += inner;
             }
 
+            onModify();
             return true;
         }
     }
@@ -774,6 +803,7 @@ bool SIM_PLOT_FRAME::updatePlot( const TRACE_DESC& aDescriptor, SIM_PLOT_PANEL*
                 data_x.data(), data_y.data(), aDescriptor.GetType() ) )
         m_workbook->AddTrace( aPanel, aDescriptor.GetTitle(), aDescriptor );
 
+    onModify();
     return true;
 }
 
@@ -891,13 +921,17 @@ bool SIM_PLOT_FRAME::loadWorkbook( const wxString& aPath )
     wxTextFile file( aPath );
 
     if( !file.Open() )
+    {
+        onModify();
         return false;
+    }
 
     long plotsCount;
 
     if( !file.GetFirstLine().ToLong( &plotsCount ) )        // GetFirstLine instead of GetNextLine
     {
         file.Close();
+        onModify();
         return false;
     }
 
@@ -906,14 +940,15 @@ bool SIM_PLOT_FRAME::loadWorkbook( const wxString& aPath )
         long plotType, tracesCount;
 
         if( !file.GetNextLine().ToLong( &plotType ) )
+        {
+            onModify();
             return false;
+        }
 
         wxString        simCommand = file.GetNextLine();
         SIM_PANEL_BASE* plotPanel = NewPlotPanel( simCommand );
         m_workbook->SetSimCommand( plotPanel, simCommand );
         StartSimulation( m_workbook->GetSimCommand( plotPanel ) );
-        //m_plots[plotPanel].m_simCommand = simCommand;
-        //StartSimulation( m_plots[plotPanel].m_simCommand );
 
         // Perform simulation, so plots can be added with values
         do
@@ -923,7 +958,10 @@ bool SIM_PLOT_FRAME::loadWorkbook( const wxString& aPath )
         while( IsSimulationRunning() );
 
         if( !file.GetNextLine().ToLong( &tracesCount ) )
+        {
+            onModify();
             return false;
+        }
 
         for( long j = 0; j < tracesCount; ++j )
         {
@@ -933,6 +971,7 @@ bool SIM_PLOT_FRAME::loadWorkbook( const wxString& aPath )
             if( !file.GetNextLine().ToLong( &traceType ) )
             {
                 file.Close();
+                onModify();
                 return false;
             }
 
@@ -942,6 +981,7 @@ bool SIM_PLOT_FRAME::loadWorkbook( const wxString& aPath )
             if( name.IsEmpty() || param.IsEmpty() )
             {
                 file.Close();
+                onModify();
                 return false;
             }
 
@@ -950,6 +990,11 @@ bool SIM_PLOT_FRAME::loadWorkbook( const wxString& aPath )
     }
 
     file.Close();
+
+    // Successfully loading a workbook does not count as modyfying it.
+    m_workbook->ClrModified();
+
+    onModify();
     return true;
 }
 
@@ -1007,9 +1052,11 @@ bool SIM_PLOT_FRAME::saveWorkbook( const wxString& aPath )
     bool res = file.Write();
     file.Close();
 
-    // Store the path of the last saved workbook. It will be used to restore the simulation if
+    // Store the filename of the last saved workbook. It will be used to restore the simulation if
     // the frame is closed and then opened again.
-    m_simulator->Settings()->SetWorkbookPath( savePath );
+    m_simulator->Settings()->SetWorkbookFilename( wxFileName( savePath ).GetFullName() );
+    m_workbook->ClrModified();
+    onModify();
 
     return res;
 }
@@ -1048,8 +1095,10 @@ void SIM_PLOT_FRAME::menuNewPlot( wxCommandEvent& aEvent )
 
         // If the previous plot had the same type, copy the simulation command
         if( prevPlot )
+        {
             m_workbook->SetSimCommand( newPlot, m_workbook->GetSimCommand( prevPlot ) );
-            //m_plots[newPlot].m_simCommand = m_plots[prevPlot].m_simCommand;
+            onModify();
+        }
     }
 }
 
@@ -1071,21 +1120,31 @@ void SIM_PLOT_FRAME::menuOpenWorkbook( wxCommandEvent& event )
 
 void SIM_PLOT_FRAME::menuSaveWorkbook( wxCommandEvent& event )
 {
-    if( !CurrentPlot() )
+    if( !m_workbook->IsModified() )
         return;
 
-    if ( !saveWorkbook( m_simulator->Settings()->GetWorkbookPath() ) )
+    wxString filename = m_simulator->Settings()->GetWorkbookFilename();
+
+    if( filename.IsEmpty() )
+    {
+        menuSaveWorkbookAs( event );
+        return;
+    }
+
+    if ( !saveWorkbook( m_simulator->Settings()->GetWorkbookFilename() ) )
         DisplayErrorMessage( this, _( "There was an error while saving the workbook file" ) );
 }
 
 
 void SIM_PLOT_FRAME::menuSaveWorkbookAs( wxCommandEvent& event )
 {
-    if( !CurrentPlot() )
-        return;
+    wxString defaultFilename = m_simulator->Settings()->GetWorkbookFilename();
 
-    wxFileDialog saveAsDlg( this, _( "Save Simulation Workbook As" ), m_savedWorkbooksPath, "",
-                            WorkbookFileWildcard(), wxFD_SAVE | wxFD_OVERWRITE_PROMPT );
+    if( defaultFilename.IsEmpty() )
+        defaultFilename = Prj().GetProjectName() + wxT( ".wbk" );
+
+    wxFileDialog saveAsDlg( this, _( "Save Simulation Workbook As" ), m_savedWorkbooksPath,
+            defaultFilename, WorkbookFileWildcard(), wxFD_SAVE | wxFD_OVERWRITE_PROMPT );
 
     if( saveAsDlg.ShowModal() == wxID_CANCEL )
         return;
@@ -1261,9 +1320,9 @@ void SIM_PLOT_FRAME::onPlotClose( wxAuiNotebookEvent& event )
             dynamic_cast<SIM_PANEL_BASE*>( m_plotNotebook->GetPage( idx ) );
 
     m_workbook->RemovePlotPanel( plotPanel );
-    updateSignalList();
     wxCommandEvent dummy;
     onCursorUpdate( dummy );
+    onModify();
 }
 
 
@@ -1356,6 +1415,7 @@ void SIM_PLOT_FRAME::onSettings( wxCommandEvent& event )
 
         m_workbook->SetSimCommand( plotPanelWindow, newCommand );
         m_simulator->Init();
+        onModify();
     }
 }
 
@@ -1465,6 +1525,25 @@ void SIM_PLOT_FRAME::onShowNetlist( wxCommandEvent& event )
 }
 
 
+bool SIM_PLOT_FRAME::canCloseWindow( wxCloseEvent& aEvent )
+{
+    if( m_workbook->IsModified() )
+    {
+        wxFileName filename = m_simulator->Settings()->GetWorkbookFilename();
+
+        if( filename.GetName().IsEmpty() )
+            filename.SetFullName( Prj().GetProjectName() + wxT( ".wbk" ) );
+
+        wxString msg = _( "Save changes to \"%s\" before closing?" );
+
+        return HandleUnsavedChanges( this, wxString::Format( msg, filename.GetFullName() ), 
+                                     [&]()->bool { return saveWorkbook( filename.GetFullPath() ); } );
+    }
+
+    return true;
+}
+
+
 void SIM_PLOT_FRAME::doCloseWindow()
 {
     if( IsSimulationRunning() )
@@ -1473,13 +1552,7 @@ void SIM_PLOT_FRAME::doCloseWindow()
     // Cancel a running simProbe or simTune tool
     m_schematicFrame->GetToolManager()->RunAction( ACTIONS::cancelInteractive );
 
-    wxFileName filename = m_simulator->Settings()->GetWorkbookPath();
-    filename.SetPath( Prj().GetProjectPath() );
-
-    saveWorkbook( filename.GetFullPath() );
-
     SaveSettings( config() );
-
     Destroy();
 }
 
@@ -1581,6 +1654,7 @@ void SIM_PLOT_FRAME::onSimFinished( wxCommandEvent& aEvent )
         updateSignalList();
         plotPanel->GetPlotWin()->UpdateAll();
         plotPanel->ResetScales();
+        onModify();
     }
     else if( simType == ST_OP )
     {
diff --git a/eeschema/sim/sim_plot_frame.h b/eeschema/sim/sim_plot_frame.h
index 817dfc808b..2cd3667670 100644
--- a/eeschema/sim/sim_plot_frame.h
+++ b/eeschema/sim/sim_plot_frame.h
@@ -145,7 +145,18 @@ private:
     void initWorkbook();
 
     /**
-     * Give icons to menuitems of the main menubar
+     * Set the main window title bar text.
+     */
+    void updateTitle();
+
+    /**
+     * Update the frame to match the changes to the workbook. Should be always called after the
+     * workbook was modified.
+     */
+    void onModify();
+
+    /**
+     * Give icons to menuitems of the main menubar.
      */
     void setIconsForMenuItems();
 
@@ -269,6 +280,7 @@ private:
     void onTune( wxCommandEvent& event );
     void onShowNetlist( wxCommandEvent& event );
 
+    bool canCloseWindow( wxCloseEvent& aEvent ) override;
     void doCloseWindow() override;
 
     void onCursorUpdate( wxCommandEvent& aEvent );
diff --git a/eeschema/sim/sim_plot_frame_base.cpp b/eeschema/sim/sim_plot_frame_base.cpp
index 2c2958ce58..a0f402c615 100644
--- a/eeschema/sim/sim_plot_frame_base.cpp
+++ b/eeschema/sim/sim_plot_frame_base.cpp
@@ -22,15 +22,15 @@ SIM_PLOT_FRAME_BASE::SIM_PLOT_FRAME_BASE( wxWindow* parent, wxWindowID id, const
 	m_fileMenu->AppendSeparator();
 
 	wxMenuItem* m_openWorkbook;
-	m_openWorkbook = new wxMenuItem( m_fileMenu, wxID_OPEN, wxString( _("Open Workbook") ) , wxEmptyString, wxITEM_NORMAL );
+	m_openWorkbook = new wxMenuItem( m_fileMenu, wxID_OPEN, wxString( _("Open...") ) , wxEmptyString, wxITEM_NORMAL );
 	m_fileMenu->Append( m_openWorkbook );
 
 	wxMenuItem* m_saveWorkbook;
-	m_saveWorkbook = new wxMenuItem( m_fileMenu, wxID_SAVE, wxString( _("Save Workbook") ) , wxEmptyString, wxITEM_NORMAL );
+	m_saveWorkbook = new wxMenuItem( m_fileMenu, wxID_SAVE, wxString( _("Save") ) , wxEmptyString, wxITEM_NORMAL );
 	m_fileMenu->Append( m_saveWorkbook );
 
 	wxMenuItem* m_saveWorkbookAs;
-	m_saveWorkbookAs = new wxMenuItem( m_fileMenu, wxID_SAVE_AS, wxString( _("Save Workbook As...") ) , wxEmptyString, wxITEM_NORMAL );
+	m_saveWorkbookAs = new wxMenuItem( m_fileMenu, wxID_SAVE_AS, wxString( _("Save As...") ) , wxEmptyString, wxITEM_NORMAL );
 	m_fileMenu->Append( m_saveWorkbookAs );
 
 	m_fileMenu->AppendSeparator();
diff --git a/eeschema/sim/sim_plot_frame_base.fbp b/eeschema/sim/sim_plot_frame_base.fbp
index 61d6b3d786..18523c4aaa 100644
--- a/eeschema/sim/sim_plot_frame_base.fbp
+++ b/eeschema/sim/sim_plot_frame_base.fbp
@@ -27,7 +27,7 @@
         <property name="ui_table">UI</property>
         <property name="use_enum">0</property>
         <property name="use_microsoft_bom">0</property>
-        <object class="Frame" expanded="1">
+        <object class="Frame" expanded="0">
             <property name="aui_managed">0</property>
             <property name="aui_manager_style">wxAUI_MGR_DEFAULT</property>
             <property name="bg"></property>
@@ -54,7 +54,7 @@
             <property name="window_name">SIM_PLOT_FRAME</property>
             <property name="window_style">wxTAB_TRAVERSAL</property>
             <property name="xrc_skip_sizer">1</property>
-            <object class="wxMenuBar" expanded="1">
+            <object class="wxMenuBar" expanded="0">
                 <property name="bg"></property>
                 <property name="context_help"></property>
                 <property name="context_menu">1</property>
@@ -76,7 +76,7 @@
                 <property name="window_extra_style"></property>
                 <property name="window_name"></property>
                 <property name="window_style"></property>
-                <object class="wxMenu" expanded="1">
+                <object class="wxMenu" expanded="0">
                     <property name="label">File</property>
                     <property name="name">m_fileMenu</property>
                     <property name="permission">protected</property>
@@ -105,7 +105,7 @@
                         <property name="help"></property>
                         <property name="id">wxID_OPEN</property>
                         <property name="kind">wxITEM_NORMAL</property>
-                        <property name="label">Open Workbook</property>
+                        <property name="label">Open...</property>
                         <property name="name">m_openWorkbook</property>
                         <property name="permission">none</property>
                         <property name="shortcut"></property>
@@ -119,21 +119,21 @@
                         <property name="help"></property>
                         <property name="id">wxID_SAVE</property>
                         <property name="kind">wxITEM_NORMAL</property>
-                        <property name="label">Save Workbook</property>
+                        <property name="label">Save</property>
                         <property name="name">m_saveWorkbook</property>
                         <property name="permission">none</property>
                         <property name="shortcut"></property>
                         <property name="unchecked_bitmap"></property>
                         <event name="OnMenuSelection">menuSaveWorkbook</event>
                     </object>
-                    <object class="wxMenuItem" expanded="1">
+                    <object class="wxMenuItem" expanded="0">
                         <property name="bitmap"></property>
                         <property name="checked">0</property>
                         <property name="enabled">1</property>
                         <property name="help"></property>
                         <property name="id">wxID_SAVE_AS</property>
                         <property name="kind">wxITEM_NORMAL</property>
-                        <property name="label">Save Workbook As...</property>
+                        <property name="label">Save As...</property>
                         <property name="name">m_saveWorkbookAs</property>
                         <property name="permission">none</property>
                         <property name="shortcut"></property>
diff --git a/eeschema/sim/sim_workbook.cpp b/eeschema/sim/sim_workbook.cpp
index c0776e4866..671e426262 100644
--- a/eeschema/sim/sim_workbook.cpp
+++ b/eeschema/sim/sim_workbook.cpp
@@ -41,7 +41,7 @@ TRACE_DESC::TRACE_DESC( const NETLIST_EXPORTER_PSPICE_SIM& aExporter, const wxSt
 
 
 SIM_WORKBOOK::SIM_WORKBOOK() :
-    m_dirty( false )
+    m_flagModified( false )
 {
 }
 
@@ -57,7 +57,7 @@ void SIM_WORKBOOK::AddPlotPanel( SIM_PANEL_BASE* aPlotPanel )
     wxASSERT( m_plots.count( aPlotPanel ) == 0 );
     m_plots[aPlotPanel] = PLOT_INFO();
 
-    m_dirty = true;
+    m_flagModified = true;
 }
 
 
@@ -66,35 +66,34 @@ void SIM_WORKBOOK::RemovePlotPanel( SIM_PANEL_BASE* aPlotPanel )
     wxASSERT( m_plots.count( aPlotPanel ) == 1 );
     m_plots.erase( aPlotPanel );
 
-    m_dirty = true;
+    m_flagModified = true;
 }
 
 
 void SIM_WORKBOOK::AddTrace( const SIM_PANEL_BASE* aPlotPanel, const wxString& aName,
         const TRACE_DESC& aTrace )
 {
-    // XXX: A plot is created automatically if there is none with this name yet.
-    m_plots[aPlotPanel].m_traces.insert(
+    m_plots.at( aPlotPanel ).m_traces.insert(
             std::make_pair( aName, aTrace ) );
 
-    m_dirty = true;
+    m_flagModified = true;
 }
 
 
 void SIM_WORKBOOK::RemoveTrace( const SIM_PANEL_BASE* aPlotPanel, const wxString& aName )
 {
-    auto& traceMap = m_plots[aPlotPanel].m_traces;
+    auto& traceMap = m_plots.at( aPlotPanel ).m_traces;
     auto traceIt = traceMap.find( aName );
     wxASSERT( traceIt != traceMap.end() );
     traceMap.erase( traceIt );
 
-    m_dirty = true;
+    m_flagModified = true;
 }
 
 
 SIM_WORKBOOK::TRACE_MAP::const_iterator SIM_WORKBOOK::RemoveTrace( const SIM_PANEL_BASE* aPlotPanel,
         TRACE_MAP::const_iterator aIt )
 {
-    m_dirty = true;
+    m_flagModified = true;
     return m_plots.at( aPlotPanel ).m_traces.erase( aIt );
 }
diff --git a/eeschema/sim/sim_workbook.h b/eeschema/sim/sim_workbook.h
index 61910d6e15..423278a441 100644
--- a/eeschema/sim/sim_workbook.h
+++ b/eeschema/sim/sim_workbook.h
@@ -124,8 +124,10 @@ public:
 
     void SetSimCommand( const SIM_PANEL_BASE* aPlotPanel, const wxString& aSimCommand )
     {
+        if( m_plots.at( aPlotPanel ).m_simCommand != aSimCommand )
+            m_flagModified = true;
+
         m_plots.at( aPlotPanel ).m_simCommand = aSimCommand;
-        m_dirty = true;
     }
 
     const wxString& GetSimCommand( const SIM_PANEL_BASE* aPlotPanel ) const
@@ -140,11 +142,13 @@ public:
         return m_plots.at( aPlotPanel ).m_traces;
     }
 
-    bool IsDirty() const { return m_dirty; }
+
+    void ClrModified() { m_flagModified = false; }
+    bool IsModified() const { return m_flagModified; }
 
 private:
     ///< Dirty bit, indicates something in the workbook has changed
-    bool m_dirty;
+    bool m_flagModified;
 
     ///< Map of plot panels and associated data
     std::map<const SIM_PANEL_BASE*, PLOT_INFO> m_plots;
diff --git a/eeschema/sim/spice_settings.cpp b/eeschema/sim/spice_settings.cpp
index c5dabf47fc..5f31bc6746 100644
--- a/eeschema/sim/spice_settings.cpp
+++ b/eeschema/sim/spice_settings.cpp
@@ -35,13 +35,13 @@ SPICE_SIMULATOR_SETTINGS::SPICE_SIMULATOR_SETTINGS( JSON_SETTINGS* aParent,
                                                     const std::string& aPath ) :
     NESTED_SETTINGS( "simulator", spiceSettingsSchemaVersion, aParent, aPath )
 {
-    m_params.emplace_back( new PARAM<wxString>( "workbook_path", &m_workbookPath, "" ) );
+    m_params.emplace_back( new PARAM<wxString>( "workbook_filename", &m_workbookFilename, "" ) );
 }
 
 
 bool SPICE_SIMULATOR_SETTINGS::operator==( const SPICE_SIMULATOR_SETTINGS &aRhs ) const
 {
-    return m_workbookPath == aRhs.m_workbookPath;
+    return m_workbookFilename == aRhs.m_workbookFilename;
 }
 
 
diff --git a/eeschema/sim/spice_settings.h b/eeschema/sim/spice_settings.h
index 71ada21b92..c2e02d91e9 100644
--- a/eeschema/sim/spice_settings.h
+++ b/eeschema/sim/spice_settings.h
@@ -43,11 +43,11 @@ public:
 
     bool operator!=( const SPICE_SIMULATOR_SETTINGS& aRhs ) const { return !( *this == aRhs ); }
 
-    wxString GetWorkbookPath() const { return m_workbookPath; }
-    void SetWorkbookPath( wxString aPath ) { m_workbookPath = aPath; }
+    wxString GetWorkbookFilename() const { return m_workbookFilename; }
+    void SetWorkbookFilename( wxString aFilename ) { m_workbookFilename = aFilename; }
 
 private:
-    wxString m_workbookPath;
+    wxString m_workbookFilename;
 };
 
 /**