kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi Wayne,

Here's the updated patch. The only changes are comments and var names.

I'll keep looking at wxGrid cell tooltips. If anyone knows if we
already can do that (or WX has an easy way that's doesn't need to deal
with mouse events manually, please let me know!)

Cheers,

John
On Tue, Oct 2, 2018 at 10:04 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> Hey John,
>
> No problem.  I'll hold off until you send the revised patch.  I'm fine
> with the current patch the way that it is.  It was just the tool tip
> code that threw me off.  I thought maybe there was something missing.
>
> Cheers,
>
> Wayne
>
> On 10/02/2018 04:45 PM, John Beard wrote:
> > Hi Wayne,
> >
> > Sorry about that, I actually started with the intention of providing
> > tooltips for the wxGrids in the library manager and the paths
> > configuration dialog. However, I found adding a tooltip to a wxGrid
> > cells is not very easy, so I retreated to just updating the help text
> > and centralising logic. I evidently neglected to update the comments
> > adequately.
> >
> > I haven't given up on the tooltips, but it'll need more work to do it
> > nicely in wx. On yt bright side, hopefully if I can do it, it will be
> > helpful for decorating other wxGrids with per cell tooltips.
> >
> > I'll update the commentary on this patch.
> >
> > Cheers,
> >
> > John
> >
> > On 1 October 2018 18:58:17 BST, Wayne Stambaugh <stambaughw@xxxxxxxxx>
> > wrote:
> >
> >     Hey John,
> >
> >     I like this solution to the translation issue better than some of the
> >     other solutions that have been proposed.  I did notice that you added a
> >     function to provide the tooltip for defined env variable but you did not
> >     hook up the tooltip logic in the dialog.  Are you planning to do that at
> >     some point in the future?
> >
> >     I'm going to merge this patch if there are no objections.
> >
> >     Cheers,
> >
> >     Wayne
> >
> >     On 10/1/2018 5:35 AM, John Beard wrote:
> >
> >         Hi,
> >
> >         Small patch that:
> >
> >         * Adds KICAD_TEMPLATE_DIR and KICAD_USER_TEMPLATE_DIR to the help
> >         dialog in Configure Paths...
> >         * Documents KICAD_PTEMPLATES as deprecated.
> >         * Moves some generic functions to do with env variables to a
> >         separate
> >         file. This means env var help text is no longer embedded in a
> >         block of
> >         HTML and also we could use it to provide tool-tips or similar to
> >         other
> >         dialogs.
> >
> >         Cheers,
> >
> >         John
> >         ------------------------------------------------------------------------
> >         Mailing list: https://launchpad.net/~kicad-developers
> >         <https://launchpad.net/%7Ekicad-developers>
> >         Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >         Unsubscribe : https://launchpad.net/~kicad-developers
> >         <https://launchpad.net/%7Ekicad-developers>
> >         More help : https://help.launchpad.net/ListHelp
> >
> >     ------------------------------------------------------------------------
> >     Mailing list: https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/%7Ekicad-developers>
> >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >     Unsubscribe : https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/%7Ekicad-developers>
> >     More help   : https://help.launchpad.net/ListHelp
> >

From c89ece074d54c7e1e85e669bfb9f6b8d71cbfd95 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Fri, 28 Sep 2018 22:08:31 +0100
Subject: [PATCH 1/4] Centralise utilities for env variables.

This puts generic logic for KiCad environment vars in
one place.

Also updates the DIALOG_CONFIGURE_PATHS help to document
the new KICAD_USER_TEMPLATE_DIR and KICAD_TEMPLATE_DIR.
---
 common/CMakeLists.txt                     |   1 +
 common/dialogs/dialog_configure_paths.cpp |  62 +++----------
 common/env_vars.cpp                       | 105 ++++++++++++++++++++++
 include/dialog_configure_paths.h          |   5 --
 include/env_vars.h                        |  59 ++++++++++++
 5 files changed, 179 insertions(+), 53 deletions(-)
 create mode 100644 common/env_vars.cpp
 create mode 100644 include/env_vars.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index eccf7302f..0f1722b6b 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -271,6 +271,7 @@ set( COMMON_SRCS
     eda_pattern_match.cpp
     eda_size_ctrl.cpp
     env_paths.cpp
+    env_vars.cpp
     exceptions.cpp
     executable_names.cpp
     filename_resolver.cpp
diff --git a/common/dialogs/dialog_configure_paths.cpp b/common/dialogs/dialog_configure_paths.cpp
index 5266a47a0..eb641a2f7 100644
--- a/common/dialogs/dialog_configure_paths.cpp
+++ b/common/dialogs/dialog_configure_paths.cpp
@@ -29,6 +29,7 @@
 #include <validators.h>
 #include <html_messagebox.h>
 #include <filename_resolver.h>
+#include <env_vars.h>
 #include <widgets/wx_grid.h>
 #include <widgets/grid_text_button_helpers.h>
 
@@ -549,60 +550,25 @@ void DIALOG_CONFIGURE_PATHS::OnHelp( wxCommandEvent& event )
                       "level.  Environment variables defined at the system or user level "
                       "take precedence over the ones defined in this table.  This means the "
                       "values in this table are ignored." );
-    msg << wxT( "<br><br><b>" );
+    msg << "<br><br><b>";
     msg << _( "To ensure environment variable names are valid on all platforms, the name field "
               "will only accept upper case letters, digits, and the underscore characters." );
-    msg << wxT( "</b><br><br>" );
-    msg << _( "<b>KICAD_SYMBOL_DIR</b> is the base path of the locally installed symbol libraries." );
-    msg << wxT( "<br><br>" );
-    msg << _( "<b>KIGITHUB</b> is used by KiCad to define the URL of the repository "
-              "of the official KiCad footprint libraries." );
-    msg << wxT( "<br><br>" );
-    msg << _( "<b>KISYS3DMOD</b> is the base path of system footprint 3D "
-              "shapes (.3Dshapes folders)." );
-    msg << wxT( "<br><br>" );
-    msg << _( "<b>KISYSMOD</b> is the base path of locally installed system "
-              "footprint libraries (.pretty folders)." );
-    msg << wxT( "<br><br>" );
-    msg << _( "<b>KIPRJMOD</b> is internally defined by KiCad (cannot be edited) and is set "
-              "to the absolute path of the currently loaded project file.  This environment "
-              "variable can be used to define files and paths relative to the currently loaded "
-              "project.  For instance, ${KIPRJMOD}/libs/footprints.pretty can be defined as a "
-              "folder containing a project specific footprint library named footprints.pretty." );
-    msg << wxT( "<br><br>" );
-    msg << _( "<b>KICAD_PTEMPLATES</b> is optional and can be defined if you want to "
-              "create your own project templates folder." );
+    msg << "</b>";
 
-    HTML_MESSAGE_BOX dlg( GetParent(), _( "Environment Variable Help" ) );
-    dlg.SetDialogSizeInDU( 400, 250 );
+    for( const auto& var: GetPredefinedEnvVars() )
+    {
+        msg << "<br><br><b>" << var << "</b>";
 
-    dlg.AddHTML_Text( msg );
-    dlg.ShowModal();
-}
+        const auto desc = LookUpEnvVarHelp( var );
 
+        if( desc.size() > 0 )
+            msg << ": " << desc;
 
-bool DIALOG_CONFIGURE_PATHS::IsEnvVarImmutable( const wxString aEnvVar )
-{
-    /*
-     * TODO - Instead of defining these values here,
-     * extract them from elsewhere in the program
-     * (where they are originally defined)
-     */
-
-    static const wxString immutable[] = {
-            "KIGITHUB",
-            "KISYS3DMOD",
-            "KISYSMOD",
-            "KIPRJMOD",
-            "KICAD_PTEMPLATES",
-            "KICAD_SYMBOL_DIR"
-    };
-
-    for( const auto& ii : immutable )
-    {
-        if( aEnvVar.Cmp( ii ) == 0 )
-            return true;
     }
 
-    return false;
+    HTML_MESSAGE_BOX dlg( GetParent(), _( "Environment Variable Help" ) );
+    dlg.SetDialogSizeInDU( 400, 250 );
+
+    dlg.AddHTML_Text( msg );
+    dlg.ShowModal();
 }
diff --git a/common/env_vars.cpp b/common/env_vars.cpp
new file mode 100644
index 000000000..ee51087ed
--- /dev/null
+++ b/common/env_vars.cpp
@@ -0,0 +1,105 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <env_vars.h>
+
+#include <map>
+
+#include <common.h>
+
+using STRING_MAP = std::map<wxString, wxString>;
+
+/*
+ * List of pre-defined environment variables
+ *
+ * TODO - Instead of defining these values here,
+ * extract them from elsewhere in the program
+ * (where they are originally defined)
+ */
+static const ENV_VAR_LIST predefined_env_vars = {
+    "KIPRJMOD",
+    "KICAD_SYMBOL_DIR",
+    "KISYS3DMOD",
+    "KISYSMOD",
+    "KIGITHUB",
+    "KICAD_TEMPLATE_DIR",
+    "KICAD_USER_TEMPLATE_DIR",
+    "KICAD_PTEMPLATES",
+};
+
+
+bool IsEnvVarImmutable( const wxString& aEnvVar )
+{
+    for( const auto& s: predefined_env_vars )
+    {
+        if( s == aEnvVar )
+            return true;
+    }
+
+    return false;
+}
+
+
+const ENV_VAR_LIST& GetPredefinedEnvVars()
+{
+    return predefined_env_vars;
+}
+
+
+void initialiseEnvVarHelp( STRING_MAP& aMap )
+{
+    // Set up dynamically, as we want to be able to use _() translations,
+    // which can't be done statically
+    aMap["KISYSMOD"] =
+        _( "The base path of locally installed system "
+            "footprint libraries (.pretty folders).");
+    aMap["KISYS3DMOD"] =
+        _( "The base path of system footprint 3D shapes (.3Dshapes folders).");
+    aMap["KICAD_SYMBOL_DIR"] =
+        _( "The base path of the locally installed symbol libraries.");
+    aMap["KIGITHUB"] =
+        _( "Used by KiCad to define the URL of the repository "
+            "of the official KiCad footprint libraries.");
+    aMap["KICAD_TEMPLATE_DIR"] =
+        _( "A directory containing project templates installed with KiCad.");
+    aMap["KICAD_USER_TEMPLATE_DIR"] =
+        _( "Optional. Can be defined if you want to create your own project "
+           "templates folder.");
+    aMap["KIPRJMOD"] =
+        _("Internally defined by KiCad (cannot be edited) and is set "
+          "to the absolute path of the currently loaded project file.  This environment "
+          "variable can be used to define files and paths relative to the currently loaded "
+          "project.  For instance, ${KIPRJMOD}/libs/footprints.pretty can be defined as a "
+          "folder containing a project specific footprint library named footprints.pretty." );
+
+    // Deprecated vars
+    aMap["KICAD_PTEMPLATES"] =
+        _( "Deprecated version of KICAD_TEMPLATE_DIR.");
+}
+
+
+wxString LookUpEnvVarHelp( const wxString& aEnvVar )
+{
+    static STRING_MAP env_var_help_text;
+
+    if( env_var_help_text.size() == 0 )
+        initialiseEnvVarHelp( env_var_help_text );
+
+    return env_var_help_text[aEnvVar];
+}
\ No newline at end of file
diff --git a/include/dialog_configure_paths.h b/include/dialog_configure_paths.h
index 311cf5da1..1d0a0153f 100644
--- a/include/dialog_configure_paths.h
+++ b/include/dialog_configure_paths.h
@@ -62,11 +62,6 @@ protected:
     void AppendEnvVar( const wxString& aName, const wxString& aPath, bool isExternal );
     void AppendSearchPath( const wxString& aName, const wxString& aPath, const wxString& aDesc );
 
-    /**
-     * Determine if a particular ENV_VAR is protected
-     */
-    bool IsEnvVarImmutable( const wxString aEnvVar );
-
 private:
     wxString            m_errorMsg;
     wxGrid*             m_errorGrid;
diff --git a/include/env_vars.h b/include/env_vars.h
new file mode 100644
index 000000000..cec25c2e0
--- /dev/null
+++ b/include/env_vars.h
@@ -0,0 +1,59 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/**
+ * @file env_vars.h
+ * Functions to provide helpful hints about what environment vars do
+ */
+
+#ifndef ENV_VARS_H
+#define ENV_VARS_H
+
+#include <wx/string.h>
+#include <vector>
+
+using ENV_VAR_LIST = std::vector<wxString>;
+
+/**
+ * Determine if an environment variable is "predefined", i.e. if the
+ * name of the variable is special to KiCad, and isn't just a user-specified
+ * substitution name.
+ * @param  aEnvVar the variable to check
+ * @return         true if predefined
+ */
+bool IsEnvVarImmutable( const wxString& aEnvVar );
+
+/**
+ * Get the list of pre-defined environment variables.
+ */
+const ENV_VAR_LIST& GetPredefinedEnvVars();
+
+/**
+ * Look up long-form help text for a given environment variable.
+ *
+ * This is intended for use in more verbose help resources (as opposed to
+ * tooltip text)
+ *
+ * @param  aEnvVar The variable to look up
+ * @return         A string with help for that variable. Empty if
+ *                 no help available for this variable.
+ */
+wxString LookUpEnvVarHelp( const wxString& aEnvVar );
+
+#endif /* ENV_VARS_H */
-- 
2.19.0