From 6e7ce095727cb639e835c11a70a2bc3cf492a8b3 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 19 Sep 2021 14:45:27 +0100 Subject: [PATCH] Use more descriptive terminology for preferred text editor. Also improves execution to allow the preferred editor to include parameters, such as "/usr/bin/open -e". Fixes https://gitlab.com/kicad/code/kicad/issues/9131 --- common/dialogs/panel_common_settings.cpp | 8 +++--- common/gestfich.cpp | 31 +++++++++++++-------- common/pgm_base.cpp | 25 ++++++++--------- common/settings/common_settings.cpp | 9 ++++-- cvpcb/dialogs/dialog_config_equfiles.cpp | 6 ++-- eeschema/dialogs/dialog_bom.cpp | 6 ++-- eeschema/sch_edit_frame.cpp | 4 +-- gerbview/tools/gerbview_inspection_tool.cpp | 6 ++-- include/gestfich.h | 20 ++++--------- include/pgm_base.h | 24 ++++++---------- include/settings/common_settings.h | 2 +- kicad/kicad_manager_frame.cpp | 2 +- kicad/project_tree_pane.cpp | 9 ++++-- kicad/tools/kicad_manager_control.cpp | 6 ++-- pcbnew/pcb_edit_frame.cpp | 2 +- 15 files changed, 79 insertions(+), 81 deletions(-) diff --git a/common/dialogs/panel_common_settings.cpp b/common/dialogs/panel_common_settings.cpp index 5b75604dc2..7259c875e3 100644 --- a/common/dialogs/panel_common_settings.cpp +++ b/common/dialogs/panel_common_settings.cpp @@ -235,7 +235,7 @@ bool PANEL_COMMON_SETTINGS::TransferDataToWindow() applySettingsToPanel( *commonSettings ); // TODO(JE) Move these into COMMON_SETTINGS probably - m_textEditorPath->SetValue( Pgm().GetEditorName( false ) ); + m_textEditorPath->SetValue( Pgm().GetTextEditor( false ) ); m_defaultPDFViewer->SetValue( Pgm().UseSystemPdfBrowser() ); m_otherPDFViewer->SetValue( !Pgm().UseSystemPdfBrowser() ); m_PDFViewerPath->SetValue( Pgm().GetPdfBrowserName() ); @@ -289,7 +289,7 @@ bool PANEL_COMMON_SETTINGS::TransferDataFromWindow() commonSettings->m_Session.remember_open_files = m_cbRememberOpenFiles->GetValue(); - Pgm().SetEditorName( m_textEditorPath->GetValue() ); + Pgm().SetTextEditor( m_textEditorPath->GetValue()); Pgm().SetPdfBrowserName( m_PDFViewerPath->GetValue() ); Pgm().ForceSystemPdfBrowser( m_defaultPDFViewer->GetValue() ); @@ -310,7 +310,7 @@ void PANEL_COMMON_SETTINGS::ResetPanel() applySettingsToPanel( defaultSettings ); // TODO(JE) Move these into COMMON_SETTINGS probably - m_textEditorPath->SetValue( defaultSettings.m_System.editor_name ); + m_textEditorPath->SetValue( defaultSettings.m_System.text_editor ); m_defaultPDFViewer->SetValue( defaultSettings.m_System.use_system_pdf_viewer ); m_otherPDFViewer->SetValue( !defaultSettings.m_System.use_system_pdf_viewer ); m_PDFViewerPath->SetValue( defaultSettings.m_System.pdf_viewer_name ); @@ -430,7 +430,7 @@ void PANEL_COMMON_SETTINGS::OnTextEditorClick( wxCommandEvent& event ) // Ask the user to select a new editor, but suggest the current one as the default. wxString editorname = Pgm().AskUserForPreferredEditor( m_textEditorPath->GetValue() ); - // If we have a new editor name request it to be copied to m_editor_name and saved + // If we have a new editor name request it to be copied to m_text_editor and saved // to the preferences file. If the user cancelled the dialog then the previous // value will be retained. if( !editorname.IsEmpty() ) diff --git a/common/gestfich.cpp b/common/gestfich.cpp index 01f4d857e2..31b540565f 100644 --- a/common/gestfich.cpp +++ b/common/gestfich.cpp @@ -39,7 +39,7 @@ #include #include -void AddDelimiterString( wxString& string ) +void QuoteString( wxString& string ) { if( !string.StartsWith( wxT( "\"" ) ) ) { @@ -112,22 +112,34 @@ wxString FindKicadFile( const wxString& shortname ) } -int ExecuteFile( wxWindow* frame, const wxString& ExecFile, const wxString& param, - wxProcess *callback ) +int ExecuteFile( const wxString& ExecFile, const wxString& param, wxProcess *callback ) { - wxString fullFileName = FindKicadFile( ExecFile ); + wxString fullFileName; + wxString fullParams; + int space = ExecFile.Find( ' ' ); + + if( space > 0 && !ExecFile.Contains( "\"" ) && !ExecFile.Contains( "\'" ) ) + { + fullFileName = FindKicadFile( ExecFile.Mid( 0, space ) ); + fullParams = ExecFile.Mid( space + 1 ) + wxS( " " ) + param; + } + else + { + fullFileName = FindKicadFile( ExecFile ); + fullParams = param; + } if( wxFileExists( fullFileName ) ) { - if( !param.IsEmpty() ) - fullFileName += wxT( " " ) + param; + if( !fullParams.IsEmpty() ) + fullFileName += wxS( " " ) + fullParams; return ProcessExecute( fullFileName, wxEXEC_ASYNC, callback ); } #ifdef __WXMAC__ else { - AddDelimiterString( fullFileName ); + QuoteString( fullFileName ); if( !param.IsEmpty() ) fullFileName += wxT( " " ) + param; @@ -135,11 +147,6 @@ int ExecuteFile( wxWindow* frame, const wxString& ExecFile, const wxString& para return ProcessExecute( wxT( "/usr/bin/open -a " ) + fullFileName, wxEXEC_ASYNC, callback ); } #endif - - wxString msg; - msg.Printf( _( "Command '%s' could not be found." ), fullFileName ); - DisplayError( frame, msg, 20 ); - return -1; } diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp index 628edbc16b..f2cf3f365b 100644 --- a/common/pgm_base.cpp +++ b/common/pgm_base.cpp @@ -135,17 +135,16 @@ wxApp& PGM_BASE::App() } -void PGM_BASE::SetEditorName( const wxString& aFileName ) +void PGM_BASE::SetTextEditor( const wxString& aFileName ) { - m_editor_name = aFileName; - wxASSERT( GetCommonSettings() ); - GetCommonSettings()->m_System.editor_name = aFileName; + m_text_editor = aFileName; + GetCommonSettings()->m_System.text_editor = aFileName; } -const wxString& PGM_BASE::GetEditorName( bool aCanShowFileChooser ) +const wxString& PGM_BASE::GetTextEditor( bool aCanShowFileChooser ) { - wxString editorname = m_editor_name; + wxString editorname = m_text_editor; if( !editorname ) { @@ -153,7 +152,7 @@ const wxString& PGM_BASE::GetEditorName( bool aCanShowFileChooser ) { // If there is no EDITOR variable set, try the desktop default #ifdef __WXMAC__ - editorname = "/usr/bin/open"; + editorname = "/usr/bin/open -e"; #elif __WXX11__ editorname = "/usr/bin/xdg-open"; #endif @@ -163,19 +162,19 @@ const wxString& PGM_BASE::GetEditorName( bool aCanShowFileChooser ) // If we still don't have an editor name show a dialog asking the user to select one if( !editorname && aCanShowFileChooser ) { - DisplayInfoMessage( nullptr, _( "No default editor found, you must choose it" ) ); + DisplayInfoMessage( nullptr, _( "No default editor found, you must choose one." ) ); editorname = AskUserForPreferredEditor(); } - // If we finally have a new editor name request it to be copied to m_editor_name and + // If we finally have a new editor name request it to be copied to m_text_editor and // saved to the preferences file. if( !editorname.IsEmpty() ) - SetEditorName( editorname ); + SetTextEditor( editorname ); - // m_editor_name already has the same value that editorname, or empty if no editor was + // m_text_editor already has the same value that editorname, or empty if no editor was // found/chosen. - return m_editor_name; + return m_text_editor; } @@ -344,7 +343,7 @@ bool PGM_BASE::setExecutablePath() void PGM_BASE::loadCommonSettings() { - m_editor_name = GetCommonSettings()->m_System.editor_name; + m_text_editor = GetCommonSettings()->m_System.text_editor; for( const std::pair it : GetCommonSettings()->m_Env.vars ) { diff --git a/common/settings/common_settings.cpp b/common/settings/common_settings.cpp index a73a61763a..19428ce565 100644 --- a/common/settings/common_settings.cpp +++ b/common/settings/common_settings.cpp @@ -280,8 +280,13 @@ COMMON_SETTINGS::COMMON_SETTINGS() : m_params.emplace_back( new PARAM( "system.autosave_interval", &m_System.autosave_interval, 600 ) ); - m_params.emplace_back( new PARAM( "system.editor_name", - &m_System.editor_name, "" ) ); +#ifdef __WXMAC__ + m_params.emplace_back( new PARAM( "system.text_editor", + &m_System.text_editor, "/usr/bin/open -e" ) ); +#else + m_params.emplace_back( new PARAM( "system.text_editor", + &m_System.text_editor, "" ) ); +#endif m_params.emplace_back( new PARAM( "system.file_history_size", &m_System.file_history_size, 9 ) ); diff --git a/cvpcb/dialogs/dialog_config_equfiles.cpp b/cvpcb/dialogs/dialog_config_equfiles.cpp index 9b06de192e..3ef9b20c4f 100644 --- a/cvpcb/dialogs/dialog_config_equfiles.cpp +++ b/cvpcb/dialogs/dialog_config_equfiles.cpp @@ -96,11 +96,11 @@ void DIALOG_CONFIG_EQUFILES::Init() void DIALOG_CONFIG_EQUFILES::OnEditEquFile( wxCommandEvent& event ) { - wxString editorname = Pgm().GetEditorName(); + wxString editorname = Pgm().GetTextEditor(); if( editorname.IsEmpty() ) { - wxMessageBox( _( "No editor defined in KiCad. Please choose it." ) ); + wxMessageBox( _( "No text editor selected in KiCad. Please choose one." ) ); return; } @@ -116,7 +116,7 @@ void DIALOG_CONFIG_EQUFILES::OnEditEquFile( wxCommandEvent& event ) m_ListChanged = true; } - ExecuteFile( this, editorname, fullFileNames ); + ExecuteFile( editorname, fullFileNames ); } diff --git a/eeschema/dialogs/dialog_bom.cpp b/eeschema/dialogs/dialog_bom.cpp index d689378146..8eaca890e8 100644 --- a/eeschema/dialogs/dialog_bom.cpp +++ b/eeschema/dialogs/dialog_bom.cpp @@ -440,11 +440,11 @@ void DIALOG_BOM::OnEditGenerator( wxCommandEvent& event ) return; } - AddDelimiterString( pluginFile ); - wxString editorname = Pgm().GetEditorName(); + QuoteString( pluginFile ); + wxString editorname = Pgm().GetTextEditor(); if( !editorname.IsEmpty() ) - ExecuteFile( this, editorname, pluginFile ); + ExecuteFile( editorname, pluginFile ); else wxMessageBox( _( "No text editor selected in KiCad. Please choose one." ) ); } diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 31a7ff72d3..976fca7508 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -980,7 +980,7 @@ void SCH_EDIT_FRAME::OnOpenPcbnew( wxCommandEvent& event ) if( Kiface().IsSingle() ) { wxString filename = QuoteFullPath( boardfn ); - ExecuteFile( this, PCBNEW_EXE, filename ); + ExecuteFile( PCBNEW_EXE, filename ); } else { @@ -1006,7 +1006,7 @@ void SCH_EDIT_FRAME::OnOpenPcbnew( wxCommandEvent& event ) { // If we are running inside a project, it should be impossible for this case to happen wxASSERT( Kiface().IsSingle() ); - ExecuteFile( this, PCBNEW_EXE ); + ExecuteFile( PCBNEW_EXE ); } } diff --git a/gerbview/tools/gerbview_inspection_tool.cpp b/gerbview/tools/gerbview_inspection_tool.cpp index ca53162057..87505fec7e 100644 --- a/gerbview/tools/gerbview_inspection_tool.cpp +++ b/gerbview/tools/gerbview_inspection_tool.cpp @@ -160,7 +160,7 @@ int GERBVIEW_INSPECTION_TOOL::ShowSource( const TOOL_EVENT& aEvent ) if( gerber_layer ) { - wxString editorname = Pgm().GetEditorName(); + wxString editorname = Pgm().GetTextEditor(); if( !editorname.IsEmpty() ) { @@ -177,12 +177,12 @@ int GERBVIEW_INSPECTION_TOOL::ShowSource( const TOOL_EVENT& aEvent ) } else { - ExecuteFile( m_frame, editorname, QuoteFullPath( fn ) ); + ExecuteFile( editorname, QuoteFullPath( fn ) ); } } else { - wxMessageBox( _( "No editor defined. Please select one." ) ); + wxMessageBox( _( "No text editor selected in KiCad. Please choose one." ) ); } } else diff --git a/include/gestfich.h b/include/gestfich.h index ed4119946d..f09892d33e 100644 --- a/include/gestfich.h +++ b/include/gestfich.h @@ -22,16 +22,8 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -/** - * This file is part of the common library - * TODO brief description - * @file gestfich.h - * @see common.h - */ - - -#ifndef __INCLUDE__GESTFICH_H__ -#define __INCLUDE__GESTFICH_H__ 1 +#ifndef GESTFICH_H +#define GESTFICH_H #include #include @@ -69,15 +61,15 @@ void KiCopyFile( const wxString& aSrcPath, const wxString& aDestPath, wxString& /** * Call the executable file \a ExecFile with the command line parameters \a param. */ -int ExecuteFile( wxWindow* frame, const wxString& ExecFile, - const wxString& param = wxEmptyString, wxProcess* callback = nullptr ); +int ExecuteFile( const wxString& ExecFile, const wxString& param = wxEmptyString, + wxProcess* callback = nullptr ); /** * Add un " to the start and the end of string (if not already done). * * @param string string to modify. */ -void AddDelimiterString( wxString& string ); +void QuoteString( wxString& string ); /** * Search the executable file shortname in KiCad binary path and return full file @@ -103,4 +95,4 @@ wxString FindKicadFile( const wxString& shortname ); */ extern wxString QuoteFullPath( wxFileName& fn, wxPathFormat format = wxPATH_NATIVE ); -#endif /* __INCLUDE__GESTFICH_H__ */ +#endif /* GESTFICH_H */ diff --git a/include/pgm_base.h b/include/pgm_base.h index 16494b1b8f..c78d0d159c 100644 --- a/include/pgm_base.h +++ b/include/pgm_base.h @@ -130,10 +130,10 @@ public: virtual COMMON_SETTINGS* GetCommonSettings() const; - virtual void SetEditorName( const wxString& aFileName ); + virtual void SetTextEditor( const wxString& aFileName ); /** - * Return the preferred editor name. + * Return the path to the preferred text editor application. * * @param aCanShowFileChooser If no editor is currently set and this argument is * 'true' then this method will show a file chooser dialog asking for the @@ -141,7 +141,7 @@ public: * @return Returns the full path of the editor, or an empty string if no editor has * been set. */ - virtual const wxString& GetEditorName( bool aCanShowFileChooser = true ); + virtual const wxString& GetTextEditor( bool aCanShowFileChooser = true ); /** * Shows a dialog that instructs the user to select a new preferred editor. @@ -306,29 +306,21 @@ protected: */ bool setExecutablePath(); +protected: std::unique_ptr m_settings_manager; std::unique_ptr m_python_scripting; - /// full path to this program - wxString m_bin_dir; + wxString m_bin_dir; /// full path to this program + wxString m_kicad_env; /// The KICAD system environment variable. - /// The KICAD system environment variable. - wxString m_kicad_env; - - /// The current locale. wxLocale* m_locale; - - /// The current language setting. int m_language_id; - /// true to use the selected PDF browser, if exists, or false to use the default bool m_use_system_pdf_browser; + wxString m_pdf_browser; /// Filename of the app selected for browsing PDFs - /// The file name of the the program selected for browsing pdf files. - wxString m_pdf_browser; - wxString m_editor_name; - + wxString m_text_editor; }; diff --git a/include/settings/common_settings.h b/include/settings/common_settings.h index b783c6b667..709f700f53 100644 --- a/include/settings/common_settings.h +++ b/include/settings/common_settings.h @@ -109,7 +109,7 @@ public: struct SYSTEM { int autosave_interval; - wxString editor_name; + wxString text_editor; int file_history_size; wxString language; wxString pdf_viewer_name; diff --git a/kicad/kicad_manager_frame.cpp b/kicad/kicad_manager_frame.cpp index d0088b143a..d6cbf11726 100644 --- a/kicad/kicad_manager_frame.cpp +++ b/kicad/kicad_manager_frame.cpp @@ -577,7 +577,7 @@ void KICAD_MANAGER_FRAME::OnOpenFileInTextEditor( wxCommandEvent& event ) wxString filename = wxT( "\"" ); filename += dlg.GetPath() + wxT( "\"" ); - if( !dlg.GetPath().IsEmpty() && !Pgm().GetEditorName().IsEmpty() ) + if( !dlg.GetPath().IsEmpty() && !Pgm().GetTextEditor().IsEmpty() ) m_toolManager->RunAction( KICAD_MANAGER_ACTIONS::openTextEditor, true, &filename ); } diff --git a/kicad/project_tree_pane.cpp b/kicad/project_tree_pane.cpp index 1cce02d9f0..685d3600d1 100644 --- a/kicad/project_tree_pane.cpp +++ b/kicad/project_tree_pane.cpp @@ -817,10 +817,13 @@ void PROJECT_TREE_PANE::onRight( wxTreeEvent& Event ) void PROJECT_TREE_PANE::onOpenSelectedFileWithTextEditor( wxCommandEvent& event ) { - wxString editorname = Pgm().GetEditorName(); + wxString editorname = Pgm().GetTextEditor(); if( editorname.IsEmpty() ) + { + wxMessageBox( _( "No text editor selected in KiCad. Please choose one." ) ); return; + } std::vector tree_data = GetSelectedData(); @@ -829,7 +832,7 @@ void PROJECT_TREE_PANE::onOpenSelectedFileWithTextEditor( wxCommandEvent& event for( PROJECT_TREE_ITEM* item_data : tree_data ) { wxString fullFileName = item_data->GetFileName(); - AddDelimiterString( fullFileName ); + QuoteString( fullFileName ); if( !files.IsEmpty() ) files += " "; @@ -837,7 +840,7 @@ void PROJECT_TREE_PANE::onOpenSelectedFileWithTextEditor( wxCommandEvent& event files += fullFileName; } - ExecuteFile( this, editorname, files ); + ExecuteFile( editorname, files ); } diff --git a/kicad/tools/kicad_manager_control.cpp b/kicad/tools/kicad_manager_control.cpp index 5faf190589..246cc7eed6 100644 --- a/kicad/tools/kicad_manager_control.cpp +++ b/kicad/tools/kicad_manager_control.cpp @@ -751,7 +751,7 @@ int KICAD_MANAGER_CONTROL::Execute( const TOOL_EVENT& aEvent ) else if( aEvent.IsAction( &KICAD_MANAGER_ACTIONS::editDrawingSheet ) ) execFile = PL_EDITOR_EXE; else if( aEvent.IsAction( &KICAD_MANAGER_ACTIONS::openTextEditor ) ) - execFile = Pgm().GetEditorName(); + execFile = Pgm().GetTextEditor(); else if( aEvent.IsAction( &KICAD_MANAGER_ACTIONS::editOtherSch ) ) execFile = EESCHEMA_EXE; else if( aEvent.IsAction( &KICAD_MANAGER_ACTIONS::editOtherPCB ) ) @@ -777,11 +777,11 @@ int KICAD_MANAGER_CONTROL::Execute( const TOOL_EVENT& aEvent ) params = m_frame->Prj().GetProjectPath(); if( !params.empty() ) - AddDelimiterString( params ); + QuoteString( params ); TERMINATE_HANDLER* callback = new TERMINATE_HANDLER( execFile ); - long pid = ExecuteFile( m_frame, execFile, params, callback ); + long pid = ExecuteFile( execFile, params, callback ); if( pid > 0 ) { diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index edb98eff26..3097cc4b14 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -1506,7 +1506,7 @@ void PCB_EDIT_FRAME::RunEeschema() if( Kiface().IsSingle() ) { wxString filename = wxT( "\"" ) + schematic.GetFullPath( wxPATH_NATIVE ) + wxT( "\"" ); - ExecuteFile( this, EESCHEMA_EXE, filename ); + ExecuteFile( EESCHEMA_EXE, filename ); } else {