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
This commit is contained in:
Ian McInerney 2019-09-26 22:09:09 +02:00 committed by Wayne Stambaugh
parent 429c7055e9
commit b33c3a5ad8
9 changed files with 114 additions and 22 deletions

View File

@ -3,7 +3,7 @@
*
* Copyright (C) 2018 Jean-Pierre Charras, jp.charras at wanadoo.fr
* Copyright (C) 2008 Wayne Stambaugh <stambaughw@gmail.com>
* 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 <regex>
#include <wildcards_and_files_ext.h>
bool compareFileExtensions( const std::string& aExtension,
const std::vector<std::string>& 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;

View File

@ -4,7 +4,7 @@
* Copyright (C) 2018 Jean-Pierre Charras, jp.charras at wanadoo.fr
* Copyright (C) 2007-2012 SoftPLC Corporation, Dick Hollenbeck <dick@softplc.com>
* Copyright (C) 2008 Wayne Stambaugh <stambaughw@gmail.com>
* 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<std::string>& aReference, bool aCaseSensitive = false );
/**
* Build the wildcard extension file dialog wildcard filter to add to the base message dialog.
*

View File

@ -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;

View File

@ -146,10 +146,10 @@ public:
return "AutoCAD DXF";
}
const wxArrayString GetFileExtensions() const override
const std::vector<std::string> GetFileExtensions() const override
{
static wxString wildcardExt = formatWildcardExt( "dxf" );
return wxArrayString( 1, &wildcardExt );
static std::vector<std::string> exts = { "dxf" };
return exts;
}
bool Load( const wxString& aFileName ) override;

View File

@ -70,14 +70,9 @@ std::unique_ptr<GRAPHICS_IMPORT_PLUGIN> 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 ) )
if( compareFileExtensions( aExtension.ToStdString(), fileExtensions ) )
return plugin;
}
}
return {};
}

View File

@ -26,6 +26,7 @@
#ifndef GRAPHICS_IMPORT_PLUGIN_H
#define GRAPHICS_IMPORT_PLUGIN_H
#include <wildcards_and_files_ext.h>
#include <wx/arrstr.h>
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<std::string> 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;

View File

@ -43,10 +43,10 @@ public:
return "Scalable Vector Graphics";
}
const wxArrayString GetFileExtensions() const override
const std::vector<std::string> GetFileExtensions() const override
{
static wxString wildcardExt = formatWildcardExt( "svg" );
return wxArrayString( 1, &wildcardExt );
static std::vector<std::string> exts = { "svg" };
return exts;
}
/**

View File

@ -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<std::string> extensionList = { "dxf", "svg", "SCH", "^g.*" };
const static std::vector<testExtensions> 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)
*/

View File

@ -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;