From b33c3a5ad8794027b8e7c627b303a5449a137e78 Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Thu, 26 Sep 2019 22:09:09 +0200 Subject: [PATCH] pcbnew: Clean up extension handling in graphics plugins * Fix wildcard display in the file selector dialog (on GTK it would show the regex to the user) * Move the file extension comparison into a common function --- common/wildcards_and_files_ext.cpp | 27 +++++++++++- include/wildcards_and_files_ext.h | 22 +++++++++- pcbnew/import_gfx/dialog_import_gfx.cpp | 6 +-- pcbnew/import_gfx/dxf_import_plugin.h | 6 +-- pcbnew/import_gfx/graphics_import_mgr.cpp | 9 +--- pcbnew/import_gfx/graphics_import_plugin.h | 7 +-- pcbnew/import_gfx/svg_import_plugin.h | 6 +-- qa/common/test_wildcards_and_files_ext.cpp | 51 ++++++++++++++++++++++ qa/pcbnew/test_graphics_import_mgr.cpp | 2 +- 9 files changed, 114 insertions(+), 22 deletions(-) diff --git a/common/wildcards_and_files_ext.cpp b/common/wildcards_and_files_ext.cpp index 37fab403f9..5eb77757f8 100644 --- a/common/wildcards_and_files_ext.cpp +++ b/common/wildcards_and_files_ext.cpp @@ -3,7 +3,7 @@ * * Copyright (C) 2018 Jean-Pierre Charras, jp.charras at wanadoo.fr * Copyright (C) 2008 Wayne Stambaugh - * Copyright (C) 1992-2018 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 1992-2019 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 @@ -27,9 +27,34 @@ * @file wildcards_and_files_ext.cpp * Definition of file extensions used in Kicad. */ +#include #include +bool compareFileExtensions( const std::string& aExtension, + const std::vector& aReference, bool aCaseSensitive ) +{ + // Form the regular expression string by placing all possible extensions into it as alternatives + std::string regexString = "("; + bool first = true; + for( auto ext : aReference ) + { + // The | separate goes between the extensions + if( !first ) + regexString += "|"; + else + first = false; + + regexString += ext; + } + regexString += ")"; + + // Create the regex and see if it matches + std::regex extRegex( regexString, aCaseSensitive ? std::regex::ECMAScript : std::regex::icase ); + return std::regex_match( aExtension, extRegex ); +} + + wxString formatWildcardExt( const wxString& aWildcard ) { wxString wc; diff --git a/include/wildcards_and_files_ext.h b/include/wildcards_and_files_ext.h index 907a684f80..c1c70d4f75 100644 --- a/include/wildcards_and_files_ext.h +++ b/include/wildcards_and_files_ext.h @@ -4,7 +4,7 @@ * Copyright (C) 2018 Jean-Pierre Charras, jp.charras at wanadoo.fr * Copyright (C) 2007-2012 SoftPLC Corporation, Dick Hollenbeck * Copyright (C) 2008 Wayne Stambaugh - * Copyright (C) 1992-2018 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 1992-2019 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 @@ -49,6 +49,26 @@ * @{ */ + +/** + * Compare the given extension against the reference extensions to see if it matches any + * of the reference extensions. + * + * This function uses the C++ regular expression functionality to perform the comparison, + * so the reference extensions can be regular expressions of their own right. This means + * that partial searches can be made, for example ^g.* can be used to see if the first + * character of the extension is g. The reference extensions are concatenated together + * as alternatives when doing the evaluation (e.g. (dxf|svg|^g.*) ). + * + * @param aExtension is the extension to test + * @param aReference is a vector containing the extensions to test against + * @param aCaseSensitive says if the comparison should be case sensitive or not + * + * @return if the extension matches any reference extensions + */ +bool compareFileExtensions( const std::string& aExtension, + const std::vector& aReference, bool aCaseSensitive = false ); + /** * Build the wildcard extension file dialog wildcard filter to add to the base message dialog. * diff --git a/pcbnew/import_gfx/dialog_import_gfx.cpp b/pcbnew/import_gfx/dialog_import_gfx.cpp index 90029fe5c7..36a8d9533d 100644 --- a/pcbnew/import_gfx/dialog_import_gfx.cpp +++ b/pcbnew/import_gfx/dialog_import_gfx.cpp @@ -241,10 +241,10 @@ void DIALOG_IMPORT_GFX::onBrowseFiles( wxCommandEvent& event ) for( auto pluginType : m_gfxImportMgr->GetImportableFileTypes() ) { auto plugin = m_gfxImportMgr->GetPlugin( pluginType ); - const auto wildcards = plugin->GetWildcards(); + const auto extensions = plugin->GetFileExtensions(); - wildcardsDesc += "|" + plugin->GetName() + " (" + wildcards + ")|" + wildcards; - allWildcards += wildcards + ";"; + wildcardsDesc += "|" + plugin->GetName() + AddFileExtListToFilter( extensions ); + allWildcards += plugin->GetWildcards() + ";"; } wildcardsDesc = _( "All supported formats|" ) + allWildcards + wildcardsDesc; diff --git a/pcbnew/import_gfx/dxf_import_plugin.h b/pcbnew/import_gfx/dxf_import_plugin.h index e774ed2c8b..d738f6e9a2 100644 --- a/pcbnew/import_gfx/dxf_import_plugin.h +++ b/pcbnew/import_gfx/dxf_import_plugin.h @@ -146,10 +146,10 @@ public: return "AutoCAD DXF"; } - const wxArrayString GetFileExtensions() const override + const std::vector GetFileExtensions() const override { - static wxString wildcardExt = formatWildcardExt( "dxf" ); - return wxArrayString( 1, &wildcardExt ); + static std::vector exts = { "dxf" }; + return exts; } bool Load( const wxString& aFileName ) override; diff --git a/pcbnew/import_gfx/graphics_import_mgr.cpp b/pcbnew/import_gfx/graphics_import_mgr.cpp index 9be929fb9c..2d969fdc35 100644 --- a/pcbnew/import_gfx/graphics_import_mgr.cpp +++ b/pcbnew/import_gfx/graphics_import_mgr.cpp @@ -70,13 +70,8 @@ std::unique_ptr GRAPHICS_IMPORT_MGR::GetPluginByExt( auto plugin = GetPlugin( fileType ); auto fileExtensions = plugin->GetFileExtensions(); - for( const auto& fileExt : fileExtensions ) - { - wxRegEx extensions( fileExt, wxRE_ICASE ); - - if( extensions.Matches( aExtension ) ) - return plugin; - } + if( compareFileExtensions( aExtension.ToStdString(), fileExtensions ) ) + return plugin; } return {}; diff --git a/pcbnew/import_gfx/graphics_import_plugin.h b/pcbnew/import_gfx/graphics_import_plugin.h index 548a887246..2ef00c5eeb 100644 --- a/pcbnew/import_gfx/graphics_import_plugin.h +++ b/pcbnew/import_gfx/graphics_import_plugin.h @@ -26,6 +26,7 @@ #ifndef GRAPHICS_IMPORT_PLUGIN_H #define GRAPHICS_IMPORT_PLUGIN_H +#include #include class GRAPHICS_IMPORTER; @@ -56,9 +57,9 @@ public: virtual const wxString GetName() const = 0; /** - * Return a string array of the file extensions handled by this plugin. + * Return a vector of the file extensions handled by this plugin. */ - virtual const wxArrayString GetFileExtensions() const = 0; + virtual const std::vector GetFileExtensions() const = 0; /** * Return a list of wildcards that contains the file extensions @@ -76,7 +77,7 @@ public: else ret += ";"; - ret += "*." + extension; + ret += "*." + formatWildcardExt( extension ); } return ret; diff --git a/pcbnew/import_gfx/svg_import_plugin.h b/pcbnew/import_gfx/svg_import_plugin.h index 78e49b8b35..708f6aecaa 100644 --- a/pcbnew/import_gfx/svg_import_plugin.h +++ b/pcbnew/import_gfx/svg_import_plugin.h @@ -43,10 +43,10 @@ public: return "Scalable Vector Graphics"; } - const wxArrayString GetFileExtensions() const override + const std::vector GetFileExtensions() const override { - static wxString wildcardExt = formatWildcardExt( "svg" ); - return wxArrayString( 1, &wildcardExt ); + static std::vector exts = { "svg" }; + return exts; } /** diff --git a/qa/common/test_wildcards_and_files_ext.cpp b/qa/common/test_wildcards_and_files_ext.cpp index 6efdd53361..fd2a0dbbcf 100644 --- a/qa/common/test_wildcards_and_files_ext.cpp +++ b/qa/common/test_wildcards_and_files_ext.cpp @@ -70,6 +70,57 @@ static constexpr bool should_use_regex_filters() } +// Structure used to store the extensions to test and the expected comparison result +struct testExtensions +{ + std::string ext; + bool insense_result; + bool sense_result; +}; + +const static std::vector extensionList = { "dxf", "svg", "SCH", "^g.*" }; +const static std::vector testExtensionList = { + { + "dxf", + true, // Case insensitive comparison result + true // Case sensitive comparison result + }, + { + "sch", + true, // Case insensitive comparison result + false // Case sensitive comparison result + }, + { + "gbr", + true, // Case insensitive comparison result + true // Case sensitive comparison result + }, + { + "pcb", + false, // Case insensitive comparison result + false // Case sensitive comparison result + } +}; + +/** + * Check correct comparison of file names + */ +BOOST_AUTO_TEST_CASE( FileNameComparison ) +{ + for( const auto& testExt : testExtensionList ) + { + bool extPresent_insense = compareFileExtensions( testExt.ext, extensionList, false ); + bool extPresent_sense = compareFileExtensions( testExt.ext, extensionList, true ); + + BOOST_TEST_INFO( "Case insensitive test for extension " + testExt.ext ); + BOOST_CHECK_EQUAL( extPresent_insense, testExt.insense_result ); + + BOOST_TEST_INFO( "Case sensitive test for extension " + testExt.ext ); + BOOST_CHECK_EQUAL( extPresent_sense, testExt.sense_result ); + } +} + + /** * Check correct handling of filter strings (as used by WX) */ diff --git a/qa/pcbnew/test_graphics_import_mgr.cpp b/qa/pcbnew/test_graphics_import_mgr.cpp index bec3a75de7..ae437a37b7 100644 --- a/qa/pcbnew/test_graphics_import_mgr.cpp +++ b/qa/pcbnew/test_graphics_import_mgr.cpp @@ -42,7 +42,7 @@ static bool pluginHandlesExt( const GRAPHICS_IMPORT_PLUGIN& aPlugin, const std:: for( auto ext : exts ) { - std::regex ext_reg( ext.ToStdString() ); + std::regex ext_reg( ext ); if( std::regex_match( aExt, ext_reg ) ) return true;