kicad-developers team mailing list archive

["kristoffer Ödmark" <kristofferodmark90@xxxxxxxxx>]

Turns out the redundant code was not very redundant. There would have to 
be some more refactoring of the code to
get this all using the NormalizePath command in common/env_paths.cpp

I am also uncertain, since I dont understand the codebase good enough, 
to why Pgm.GetLocalEnvVars does not
include the env var to KIPRJMOD, I guess there is a reason?


On 2018-04-06 13:05, Jeff Young wrote:
> Patch looks good to me, although I’d rather see the redundant code removed.
>
>
> > On 6 Apr 2018, at 11:30, kristoffer Ödmark <kristofferodmark90@xxxxxxxxx> wrote:
> >
> > <0001-Footprint-Wizard-now-also-handles-custom-Env-paths.patch>

>From 561524c7e84d0fc3f0dca70057505e0f84d5147d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kristoffer=20=C3=96dmark?= <kristoffer.odmark90@xxxxxxxxx>
Date: Fri, 6 Apr 2018 12:06:05 +0200
Subject: [PATCH] Footprint Wizard now also handles custom Env paths

---
 common/env_paths.cpp                | 17 +++++++++++++----
 include/env_paths.h                 | 16 ++++++++++++++--
 pcbnew/dialogs/wizard_add_fplib.cpp | 19 +++++++++++--------
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/common/env_paths.cpp b/common/env_paths.cpp
index 3569c436d..9e6a6715c 100644
--- a/common/env_paths.cpp
+++ b/common/env_paths.cpp
@@ -64,9 +64,8 @@ static bool normalizeAbsolutePaths( const wxFileName& aPathA,
     return true;
 }
 
-
 wxString NormalizePath( const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars,
-        const PROJECT* aProject )
+        const wxString& aProjectPath )
 {
     wxFileName envPath;
     wxString tmp, varName, normalizedFullPath;
@@ -90,9 +89,10 @@ wxString NormalizePath( const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars
         }
     }
 
-    if( varName.IsEmpty() && aProject )
+    if( varName.IsEmpty() && !aProjectPath.IsEmpty()
+        && wxFileName( aProjectPath ).IsAbsolute() && wxFileName( aFilePath ).IsAbsolute() )
     {
-        envPath.SetPath( aProject->GetProjectPath() );
+        envPath.SetPath( aProjectPath );
 
         if( normalizeAbsolutePaths( envPath, aFilePath, &tmp ) )
             varName = PROJECT_VAR_NAME;
@@ -111,6 +111,15 @@ wxString NormalizePath( const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars
     return normalizedFullPath;
 }
 
+wxString NormalizePath( const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars,
+        const PROJECT* aProject )
+{
+  if( aProject )
+    return NormalizePath( aFilePath, aEnvVars, aProject->GetProjectPath() );
+  else
+    return NormalizePath( aFilePath, aEnvVars, "" );
+}
+
 
 // Create file path by appending path and file name. This approach allows the filename
 // to contain a relative path, whereas wxFileName::SetPath() would replace the
diff --git a/include/env_paths.h b/include/env_paths.h
index bac92ffca..a27fbcc7c 100644
--- a/include/env_paths.h
+++ b/include/env_paths.h
@@ -39,6 +39,18 @@
 wxString NormalizePath( const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars,
         const PROJECT* aProject );
 
+/**
+ * Normalizes a file path to an environmental variable, if possible.
+ *
+ * @param aFilePath is the full file path (path and file name) to be normalized.
+ * @param aEnvVars is an optional map of environmental variables to try substition with.
+ * @param aProjectPath is an optional string to normalize the file path to the project path.
+ * @return Normalized full file path (path and file name) if succeeded or empty string if the
+ *          path could not be normalized.
+ */
+wxString NormalizePath(
+        const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars, const wxString& aProjectPath );
+
 /**
  * Searches the default paths trying to find one with the requested file.
  *
@@ -48,7 +60,7 @@ wxString NormalizePath( const wxFileName& aFilePath, const ENV_VAR_MAP* aEnvVars
  * @return Full path (apth and file name) if the file was found in one of the paths, otherwise
  *      an empty string.
 */
-wxString ResolveFile( const wxString& aFileName, const ENV_VAR_MAP* aEnvVars,
-        const PROJECT* aProject );
+wxString ResolveFile(
+        const wxString& aFileName, const ENV_VAR_MAP* aEnvVars, const PROJECT* aProject );
 
 #endif /* ENV_PATHS_H */
diff --git a/pcbnew/dialogs/wizard_add_fplib.cpp b/pcbnew/dialogs/wizard_add_fplib.cpp
index 193253cb3..b34822f0f 100644
--- a/pcbnew/dialogs/wizard_add_fplib.cpp
+++ b/pcbnew/dialogs/wizard_add_fplib.cpp
@@ -46,6 +46,7 @@
 #include <bitmaps.h>
 
 #include <class_module.h>
+#include <env_paths.h>
 
 #ifdef BUILD_GITHUB_PLUGIN
 #include <../github/github_getliblist.h>
@@ -222,17 +223,14 @@ wxString WIZARD_FPLIB_TABLE::LIBRARY::GetRelativePath( const wxString& aBase, co
 
 wxString WIZARD_FPLIB_TABLE::LIBRARY::GetAutoPath( LIB_SCOPE aScope ) const
 {
-    const wxString& global_env = FP_LIB_TABLE::GlobalPathEnvVariableName();
     const wxString& project_env = PROJECT_VAR_NAME;
     const wxString& github_env( "KIGITHUB" );
 
     wxString rel_path;
 
-    // KISYSMOD check
-    rel_path = replaceEnv( global_env );
-
-    if( !rel_path.IsEmpty() )
-        return rel_path;
+    // The extra KIGITHUB and KIPRJCHECKS are still here since Pgm.GetLocalVariables() does not
+    // contain the KIPRJMOD env var, and the KIGITHUB does not pass the IsAbsolutePath check
+    // that happens in NormalizePath(...) since it starts with https://
 
     // KIGITHUB check
     rel_path = replaceEnv( github_env, false );
@@ -249,8 +247,13 @@ wxString WIZARD_FPLIB_TABLE::LIBRARY::GetAutoPath( LIB_SCOPE aScope ) const
             return rel_path;
     }
 
-    // Return the full path
-    return m_path;
+    rel_path = NormalizePath( wxFileName( m_path ), &Pgm().GetLocalEnvVariables(), project_env );
+
+    // If normalizePath failed, then rel_path will be empty, m_path is the full path.
+    if( rel_path.IsEmpty() )
+      return m_path;
+
+    return rel_path;
 }
 
 
-- 
2.16.2