Fix Preferred Editor dialog (lp: 1558353)

The functions involved in the problem were PGM_BASE::GetEditorName and
EDA_BASE_FRAME::OnSelectPreferredEditor:

1) OnSelectPreferredEditor showed a dialog to allow the user selecting the
editor, but before that called GetEditorName to get the name of the current
editor (to show as a default in the choose file dialog).

2) The problem was when there was no editor, GetEditorName showed its own
dialog.

3) So the user was seeing first the dialog from (2) and then the dialog from
(1).

4) As GetEditorName is used in many other places the solution I did was to add
to it an optional parameter that tells it what to do if no editor is set. To
avoid modifying other code that relies on the current behaviour, this parameter
has a default value that causes to show the dialog. But now when
OnSelectPreferredEditor calls it, it passes the parameter that causes it to
return an empty string if no editor was set.

5) Also, I found a second bug while doing it which allowed in the first dialog
to select an unexistent file (the dialog was missing the wxFD_FILE_MUST_EXIST
flag).

6) Lastly, to avoid having duplicated code (the one that showed the same dialog
and that configured the wildcard was in two methods) I created a single
function that now both functions call: PGM_BASE::AskUserForPreferredEditor.
This way we also will have consistency in the behaviour of both dialogs and
there is a single place where it needs to be modified.

common/basicframe.cpp

@@ -441,26 +441,18 @@ void EDA_BASE_FRAME::GetKicadHelp( wxCommandEvent& event )
 
 void EDA_BASE_FRAME::OnSelectPreferredEditor( wxCommandEvent& event )
 {
-    wxFileName  fn = Pgm().GetEditorName();
+    // Ask for the current editor and instruct GetEditorName() to not show 
-    wxString    wildcard( wxT( "*" ) );
+    // unless we pass false as argument.
+    wxString editorname = Pgm().GetEditorName( false );
 
-#ifdef __WINDOWS__
+    // Ask the user to select a new editor, but suggest the current one as the default.
-    wildcard += wxT( ".exe" );
+    editorname = Pgm().AskUserForPreferredEditor( editorname );
-#endif
 
-    wildcard.Printf( _( "Executable file (%s)|%s" ),
+    // If we have a new editor name request it to be copied to m_editor_name and saved
-                     GetChars( wildcard ), GetChars( wildcard ) );
+    // to the preferences file. If the user cancelled the dialog then the previous
-
+    // value will be retained.
-    wxFileDialog dlg( this, _( "Select Preferred Editor" ), fn.GetPath(),
+    if( !editorname.IsEmpty() )
-                      fn.GetFullName(), wildcard,
+        Pgm().SetEditorName( editorname );
-                      wxFD_OPEN | wxFD_FILE_MUST_EXIST );
-
-    if( dlg.ShowModal() == wxID_CANCEL )
-        return;
-
-    wxString editor = dlg.GetPath();
-
-    Pgm().SetEditorName( editor );
 }
 
 

common/pgm_base.cpp

@@ -315,14 +315,13 @@ void PGM_BASE::SetEditorName( const wxString& aFileName )
 }
 
 
-const wxString& PGM_BASE::GetEditorName()
+const wxString& PGM_BASE::GetEditorName( bool aCanShowFileChooser )
 {
     wxString editorname = m_editor_name;
 
     if( !editorname )
     {
-        // Get the preferred editor name from environment variable first.
+        if( !wxGetEnv( wxT( "EDITOR" ), &editorname ) )
-        if(!wxGetEnv( wxT( "EDITOR" ), &editorname ))
         {
             // If there is no EDITOR variable set, try the desktop default
 #ifdef __WXMAC__
@@ -333,30 +332,47 @@ const wxString& PGM_BASE::GetEditorName()
         }
     }
 
-    if( !editorname )       // We must get a preferred editor name
+    // If we still don't have an editor name show a dialog asking the user to select one
+    if( !editorname && aCanShowFileChooser )
     {
         DisplayInfoMessage( NULL,
                             _( "No default editor found, you must choose it" ) );
 
-        wxString mask( wxT( "*" ) );
+        editorname = AskUserForPreferredEditor();
-
-#ifdef __WINDOWS__
-        mask += wxT( ".exe" );
-#endif
-        editorname = EDA_FILE_SELECTOR( _( "Preferred Editor:" ), wxEmptyString,
-                                        wxEmptyString, wxEmptyString, mask,
-                                        NULL, wxFD_OPEN, true );
     }
 
+    // If we finally have a new editor name request it to be copied to m_editor_name and
+    // saved to the preferences file.
     if( !editorname.IsEmpty() )
-    {
+        SetEditorName( editorname );
-        m_editor_name = editorname;
-        m_common_settings->Write( wxT( "Editor" ), m_editor_name );
-    }
 
+    // m_editor_name already has the same value that editorname, or empty if no editor was
+    // found/chosen.
     return m_editor_name;
 }
 
+const wxString PGM_BASE::AskUserForPreferredEditor( const wxString& aDefaultEditor )
+{
+    // Create a mask representing the executable files in the current platform
+#ifdef __WINDOWS__
+    wxString mask( _( "Executable file (*.exe)|*.exe" ) );
+#else
+    wxString mask( _( "Executable file (*)|*" ) );
+#endif
+
+    // Extract the path, name and extension from the default editor (even if the editor's
+    // name was empty, this method will succeed and return empty strings).
+    wxString path, name, ext;
+    wxFileName::SplitPath( aDefaultEditor, &path, &name, &ext );
+
+    // Show the modal editor and return the file chosen (may be empty if the user cancels
+    // the dialog).
+    return EDA_FILE_SELECTOR( _( "Select Preferred Editor" ), path,
+                              name, ext, mask,
+                              NULL, wxFD_OPEN | wxFD_FILE_MUST_EXIST,
+                              true );
+}
+
 
 bool PGM_BASE::initPgm()
 {
@@ -603,7 +619,7 @@ void PGM_BASE::saveCommonSettings()
 
         for( ENV_VAR_MAP_ITER it = m_local_env_vars.begin(); it != m_local_env_vars.end(); ++it )
         {
-            wxLogTrace( traceEnvVars, wxT( "Saving environment varaiable config entry %s as %s" ),
+            wxLogTrace( traceEnvVars, wxT( "Saving environment variable config entry %s as %s" ),
                         GetChars( it->first ),  GetChars( it->second.GetValue() ) );
             m_common_settings->Write( it->first, it->second.GetValue() );
         }

include/pgm_base.h

@@ -133,8 +133,23 @@ public:
 
     /**
      * Return the preferred editor name.
+     * @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
+     *          editor's executable.
+     * @return  Returns the full path of the editor, or an empty string if no editor has
+     *          been set.
      */
-    VTBL_ENTRY const wxString& GetEditorName();
+    VTBL_ENTRY const wxString& GetEditorName( bool aCanShowFileChooser = true );
+    
+    /**
+     * Shows a dialog that instructs the user to select a new preferred editor.
+     * @param   aDefaultEditor Default full path for the default editor this dialog should
+     *          show by default.
+     * @return  Returns the full path of the editor, or an empty string if no editor was
+     *          chosen.
+     */
+    VTBL_ENTRY const wxString AskUserForPreferredEditor(
+                                        const wxString& aDefaultEditor = wxEmptyString );
 
     VTBL_ENTRY bool IsKicadEnvVariableDefined() const               { return !m_kicad_env.IsEmpty(); }