← Back to team overview

kicad-developers team mailing list archive

[PATCH] Wildcard tests + removed variadic interface

 

Hi,

Here are some patches for wildcards:

* Add some tests to demo the API and expected output.
* Remove a variadic function signature and replace with one that takes
a std::vector<std::string>. This allows better and more typesafe use
of this API at run time.
* Unify the "all files" wildcard handling. Avoids unexpected needs to
call translation functions and sticks to a single idiom for getting
wildcards (call one of these functions).

Could a Windows user please check `make test` works before commit? I
did check it by fiddling with the defines, but good to be sure.

Cheers,

John
From cf87f505f6c5755e8db4cfe91b1455a04bd27d03 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Tue, 8 Jan 2019 11:57:39 +0000
Subject: [PATCH 3/3] Wildcards: unify handling of all files wildcards

Use the AddFileExtListToFilter() to also generate the
wildcard for "all files". This is because:

* Users can use AddFileExtListToFilter for the all files WC
  with the same interface as for any other extensions.
* Users do not need to worry about wxGetTranslation, as the
  _() is applied in the same way as the other *Wildcard() helpers,
  and it is a function just like the others, so it is consistent
* There is a testable interface to document the expected result.
  The test is added.
---
 common/wildcards_and_files_ext.cpp         | 12 ++++++++++--
 eeschema/dialogs/dialog_netlist.cpp        |  2 +-
 eeschema/dialogs/dialog_spice_model.cpp    |  2 +-
 gerbview/files.cpp                         |  4 ++--
 include/wildcards_and_files_ext.h          |  4 ++--
 kicad/mainframe.cpp                        |  2 +-
 pcbnew/footprint_libraries_utils.cpp       |  2 +-
 qa/common/test_wildcards_and_files_ext.cpp |  5 +++++
 8 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/common/wildcards_and_files_ext.cpp b/common/wildcards_and_files_ext.cpp
index 3f9eb12f0..e7486ba90 100644
--- a/common/wildcards_and_files_ext.cpp
+++ b/common/wildcards_and_files_ext.cpp
@@ -68,9 +68,14 @@ static wxString formatWildcardExt( const wxString& aWildcard )
 #endif
 }
 
-
 wxString AddFileExtListToFilter( const std::vector<std::string>& aExts )
 {
+    if( aExts.size() == 0 )
+    {
+        // The "all files" wildcard must not exclude files without any ext
+        return " (*)|*";
+    }
+
     wxString files_filter = " (";
 
     // Add extensions to the info message:
@@ -136,7 +141,10 @@ const std::string PngFileExtension( "png" );
 const std::string JpegFileExtension( "jpg" );
 
 
-const wxString AllFilesWildcard( _( "All files (*)|*" ) );
+wxString AllFilesWildcard()
+{
+    return _( "All files" ) + AddFileExtListToFilter( {} );
+}
 
 
 wxString SchematicSymbolFileWildcard()
diff --git a/eeschema/dialogs/dialog_netlist.cpp b/eeschema/dialogs/dialog_netlist.cpp
index 4287e0d0d..b6ff574ad 100644
--- a/eeschema/dialogs/dialog_netlist.cpp
+++ b/eeschema/dialogs/dialog_netlist.cpp
@@ -654,7 +654,7 @@ bool NETLIST_DIALOG::FilenamePrms( NETLIST_TYPE_ID aNetTypeId,
         break;
 
     default:    // custom, NET_TYPE_CUSTOM1 and greater
-        fileWildcard = AllFilesWildcard;
+        fileWildcard = AllFilesWildcard();
         ret = false;
     }
 
diff --git a/eeschema/dialogs/dialog_spice_model.cpp b/eeschema/dialogs/dialog_spice_model.cpp
index d9d1f3f08..5a6cd6b20 100644
--- a/eeschema/dialogs/dialog_spice_model.cpp
+++ b/eeschema/dialogs/dialog_spice_model.cpp
@@ -764,7 +764,7 @@ void DIALOG_SPICE_MODEL::onSelectLibrary( wxCommandEvent& event )
     if( searchPath.IsEmpty() )
         searchPath = Prj().GetProjectPath();
 
-    wxString wildcards = SpiceLibraryFileWildcard() + "|" + AllFilesWildcard;
+    wxString     wildcards = SpiceLibraryFileWildcard() + "|" + AllFilesWildcard();
     wxFileDialog openDlg( this, _( "Select library" ), searchPath, "", wildcards,
             wxFD_OPEN | wxFD_FILE_MUST_EXIST );
 
diff --git a/gerbview/files.cpp b/gerbview/files.cpp
index e83c33406..752afd700 100644
--- a/gerbview/files.cpp
+++ b/gerbview/files.cpp
@@ -219,7 +219,7 @@ bool GERBVIEW_FRAME::LoadGerberFiles( const wxString& aFullFileName )
         filetypes += _( "Bottom Pad Master (*.GPB)|*.GPB;*.gpb|" );
 
         // All filetypes
-        filetypes += AllFilesWildcard;
+        filetypes += AllFilesWildcard();
 
         // Use the current working directory if the file name path does not exist.
         if( filename.DirExists() )
@@ -391,7 +391,7 @@ bool GERBVIEW_FRAME::LoadExcellonFiles( const wxString& aFullFileName )
         filetypes << wxT( "|" );
 
         /* All filetypes */
-        filetypes += wxGetTranslation( AllFilesWildcard );
+        filetypes += AllFilesWildcard();
 
         /* Use the current working directory if the file name path does not exist. */
         if( filename.DirExists() )
diff --git a/include/wildcards_and_files_ext.h b/include/wildcards_and_files_ext.h
index f48588a77..3454084a0 100644
--- a/include/wildcards_and_files_ext.h
+++ b/include/wildcards_and_files_ext.h
@@ -62,7 +62,7 @@
  * are all match if you pass "txt" into the function).
  *
  * @param aExts is the list of exts to add to the filter. Do not include the
- * leading dot.
+ * leading dot. Empty means "allow all files".
  *
  * @return the appropriate file dialog wildcard filter list.
  */
@@ -127,7 +127,7 @@ extern const std::string JpegFileExtension;
  * @{
  */
 
-extern const wxString AllFilesWildcard;
+extern wxString AllFilesWildcard();
 
 extern wxString ComponentFileWildcard();
 extern wxString PageLayoutDescrFileWildcard();
diff --git a/kicad/mainframe.cpp b/kicad/mainframe.cpp
index 709d52b47..4d4ec27f4 100644
--- a/kicad/mainframe.cpp
+++ b/kicad/mainframe.cpp
@@ -471,7 +471,7 @@ void KICAD_MANAGER_FRAME::OnOpenTextEditor( wxCommandEvent& event )
 void KICAD_MANAGER_FRAME::OnOpenFileInTextEditor( wxCommandEvent& event )
 {
     // show all files in file dialog (in Kicad all files are editable texts):
-    wxString wildcard = AllFilesWildcard;
+    wxString wildcard = AllFilesWildcard();
 
     wxString default_dir = Prj().GetProjectPath();
 
diff --git a/pcbnew/footprint_libraries_utils.cpp b/pcbnew/footprint_libraries_utils.cpp
index 2942666a9..e338f4df2 100644
--- a/pcbnew/footprint_libraries_utils.cpp
+++ b/pcbnew/footprint_libraries_utils.cpp
@@ -105,7 +105,7 @@ static wxFileName getFootprintFilenameFromUser( wxWindow* aParent, const wxStrin
     wildCard << KiCadFootprintLibFileWildcard() << wxChar( '|' )
              << ModLegacyExportFileWildcard() << wxChar( '|' )
              << GedaPcbFootprintLibFileWildcard() << wxChar( '|' )
-             << AllFilesWildcard;
+             << AllFilesWildcard();
 
     wxFileDialog dlg( aParent, FMT_IMPORT_MODULE, aLastPath, wxEmptyString, wildCard,
             wxFD_OPEN | wxFD_FILE_MUST_EXIST );
diff --git a/qa/common/test_wildcards_and_files_ext.cpp b/qa/common/test_wildcards_and_files_ext.cpp
index 78e88c59e..a4a815185 100644
--- a/qa/common/test_wildcards_and_files_ext.cpp
+++ b/qa/common/test_wildcards_and_files_ext.cpp
@@ -47,6 +47,11 @@ struct ExtWildcardFilterCase
 };
 
 const static std::vector<ExtWildcardFilterCase> ext_wildcard_cases = {
+    {
+            {},
+            " (*)|*",
+            " (*)|*",
+    },
     {
             { "png" },
             " ( *.png)|*.png",
-- 
2.20.1

From d71e19f2b62bf5abb9ba8ec01b03e93389fc31bb Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Sat, 5 Jan 2019 23:55:14 +0000
Subject: [PATCH 2/3] Wildcards: use vectors instead of varargs + tests

Varargs make it very hard to pass strings in flexibly
when they (and the number of them) are not known
at compile time.

For example, this allows up to now amalgamate the "single
ext" and "multiple ext" unit tests.
---
 common/wildcards_and_files_ext.cpp         | 111 ++++++++++-----------
 include/wildcards_and_files_ext.h          |  17 +++-
 qa/common/test_wildcards_and_files_ext.cpp |  25 ++---
 3 files changed, 72 insertions(+), 81 deletions(-)

diff --git a/common/wildcards_and_files_ext.cpp b/common/wildcards_and_files_ext.cpp
index 5dac72643..3f9eb12f0 100644
--- a/common/wildcards_and_files_ext.cpp
+++ b/common/wildcards_and_files_ext.cpp
@@ -69,37 +69,30 @@ static wxString formatWildcardExt( const wxString& aWildcard )
 }
 
 
-wxString AddFileExtListToFilter( int aArgCnt, ... )
+wxString AddFileExtListToFilter( const std::vector<std::string>& aExts )
 {
     wxString files_filter = " (";
 
-    va_list args;
-    va_start( args, aArgCnt );
-
     // Add extensions to the info message:
-    for( int ii = 0; ii < aArgCnt; ii++)
+    for( const auto& ext : aExts )
     {
-        const char* ext = va_arg(args, char*);
         files_filter << " *." << ext;
     }
 
     files_filter << ")|*.";
 
-    va_start( args, aArgCnt );
-
     // Add extensions to the filter list, using a formated string (GTK specific):
-    for( int ii = 0; ii < aArgCnt; ii++)
+    bool first = true;
+    for( const auto& ext : aExts )
     {
-        const char* ext = va_arg( args, const char* );
-
-        if( ii > 0 )
+        if( !first )
             files_filter << ";*.";
 
+        first = false;
+
         files_filter << formatWildcardExt( ext );
     }
 
-    va_end( args );
-
     return files_filter;
 }
 
@@ -148,271 +141,271 @@ const wxString AllFilesWildcard( _( "All files (*)|*" ) );
 
 wxString SchematicSymbolFileWildcard()
 {
-    return _( "KiCad drawing symbol files" ) + AddFileExtListToFilter( 1, "sym" );
+    return _( "KiCad drawing symbol files" ) + AddFileExtListToFilter( { "sym" } );
 }
 
 
 wxString SchematicLibraryFileWildcard()
 {
-    return _( "KiCad symbol library files" ) + AddFileExtListToFilter( 1, "lib" );
+    return _( "KiCad symbol library files" ) + AddFileExtListToFilter( { "lib" } );
 }
 
 
 wxString ProjectFileWildcard()
 {
-    return _( "KiCad project files" ) + AddFileExtListToFilter( 1, "pro" );
+    return _( "KiCad project files" ) + AddFileExtListToFilter( { "pro" } );
 }
 
 
 wxString SchematicFileWildcard()
 {
-    return _( "KiCad schematic files" ) + AddFileExtListToFilter( 1, "sch" );
+    return _( "KiCad schematic files" ) + AddFileExtListToFilter( { "sch" } );
 }
 
 
 wxString EagleSchematicFileWildcard()
 {
-    return _( "Eagle XML schematic files" ) + AddFileExtListToFilter( 1, "sch" );
+    return _( "Eagle XML schematic files" ) + AddFileExtListToFilter( { "sch" } );
 }
 
 
 wxString EagleFilesWildcard()
 {
-    return _( "Eagle XML files" ) + AddFileExtListToFilter( 2, "sch", "brd" );
+    return _( "Eagle XML files" ) + AddFileExtListToFilter( { "sch", "brd" } );
 }
 
 
 wxString NetlistFileWildcard()
 {
-    return _( "KiCad netlist files" ) + AddFileExtListToFilter( 1, "net" );
+    return _( "KiCad netlist files" ) + AddFileExtListToFilter( { "net" } );
 }
 
 
 wxString GerberFileWildcard()
 {
-    return _( "Gerber files" ) + AddFileExtListToFilter( 1, "pho" );
+    return _( "Gerber files" ) + AddFileExtListToFilter( { "pho" } );
 }
 
 
 wxString LegacyPcbFileWildcard()
 {
-    return _( "KiCad printed circuit board files" ) + AddFileExtListToFilter( 1, "brd" );
+    return _( "KiCad printed circuit board files" ) + AddFileExtListToFilter( { "brd" } );
 }
 
 
 wxString EaglePcbFileWildcard()
 {
-    return _( "Eagle ver. 6.x XML PCB files" ) + AddFileExtListToFilter( 1, "brd" );
+    return _( "Eagle ver. 6.x XML PCB files" ) + AddFileExtListToFilter( { "brd" } );
 }
 
 
 wxString PCadPcbFileWildcard()
 {
-    return _( "P-Cad 200x ASCII PCB files" ) + AddFileExtListToFilter( 1, "pcb" );
+    return _( "P-Cad 200x ASCII PCB files" ) + AddFileExtListToFilter( { "pcb" } );
 }
 
 
 wxString PcbFileWildcard()
 {
-    return _( "KiCad printed circuit board files" ) + AddFileExtListToFilter( 1, "kicad_pcb" );
+    return _( "KiCad printed circuit board files" ) + AddFileExtListToFilter( { "kicad_pcb" } );
 }
 
 
 wxString KiCadFootprintLibFileWildcard()
 {
-    return _( "KiCad footprint files" ) + AddFileExtListToFilter( 1, "kicad_mod" );
+    return _( "KiCad footprint files" ) + AddFileExtListToFilter( { "kicad_mod" } );
 }
 
 
 wxString KiCadFootprintLibPathWildcard()
 {
-    return _( "KiCad footprint library paths" ) + AddFileExtListToFilter( 1, "pretty" );
+    return _( "KiCad footprint library paths" ) + AddFileExtListToFilter( { "pretty" } );
 }
 
 
 wxString LegacyFootprintLibPathWildcard()
 {
-    return _( "Legacy footprint library files" ) + AddFileExtListToFilter( 1, "mod" );
+    return _( "Legacy footprint library files" ) + AddFileExtListToFilter( { "mod" } );
 }
 
 
 wxString EagleFootprintLibPathWildcard()
 {
-    return _( "Eagle ver. 6.x XML library files" ) + AddFileExtListToFilter( 1, "lbr" );
+    return _( "Eagle ver. 6.x XML library files" ) + AddFileExtListToFilter( { "lbr" } );
 }
 
 
 wxString GedaPcbFootprintLibFileWildcard()
 {
-    return _( "Geda PCB footprint library files" ) + AddFileExtListToFilter( 1, "fp" );
+    return _( "Geda PCB footprint library files" ) + AddFileExtListToFilter( { "fp" } );
 }
 
 
 wxString PageLayoutDescrFileWildcard()
 {
-    return _( "Page layout design files" ) + AddFileExtListToFilter( 1, "kicad_wks" );
+    return _( "Page layout design files" ) + AddFileExtListToFilter( { "kicad_wks" } );
 }
 
 
 // Wildcard for cvpcb component to footprint link file
 wxString ComponentFileWildcard()
 {
-    return _( "KiCad symbol footprint link files" ) + AddFileExtListToFilter( 1, "cmp" );
+    return _( "KiCad symbol footprint link files" ) + AddFileExtListToFilter( { "cmp" } );
 }
 
 
 // Wildcard for reports and fabrication documents
 wxString DrillFileWildcard()
 {
-    return _( "Drill files" ) + AddFileExtListToFilter( 3, "drl", "nc", "xnc" );
+    return _( "Drill files" ) + AddFileExtListToFilter( { "drl", "nc", "xnc" } );
 }
 
 
 wxString SVGFileWildcard()
 {
-    return _( "SVG files" ) + AddFileExtListToFilter( 1, "svg" );
+    return _( "SVG files" ) + AddFileExtListToFilter( { "svg" } );
 }
 
 
 wxString HtmlFileWildcard()
 {
-    return _( "HTML files" ) + AddFileExtListToFilter( 2, "htm" , "html" );
+    return _( "HTML files" ) + AddFileExtListToFilter( { "htm", "html" } );
 }
 
 
 wxString CsvFileWildcard()
 {
-    return _( "CSV Files" ) + AddFileExtListToFilter( 1, "csv" );
+    return _( "CSV Files" ) + AddFileExtListToFilter( { "csv" } );
 }
 
 
 wxString PdfFileWildcard()
 {
-    return _( "Portable document format files" ) + AddFileExtListToFilter( 1, "pdf" );
+    return _( "Portable document format files" ) + AddFileExtListToFilter( { "pdf" } );
 }
 
 
 wxString PSFileWildcard()
 {
-    return _( "PostScript files" ) + AddFileExtListToFilter( 1, "ps" );
+    return _( "PostScript files" ) + AddFileExtListToFilter( { "ps" } );
 }
 
 
 wxString ReportFileWildcard()
 {
-    return _( "Report files" ) + AddFileExtListToFilter( 1, "rpt" );
+    return _( "Report files" ) + AddFileExtListToFilter( { "rpt" } );
 }
 
 
 wxString FootprintPlaceFileWildcard()
 {
-    return _( "Footprint place files" ) + AddFileExtListToFilter( 1, "pos" );
+    return _( "Footprint place files" ) + AddFileExtListToFilter( { "pos" } );
 }
 
 
 wxString Shapes3DFileWildcard()
 {
-    return _( "VRML and X3D files" ) + AddFileExtListToFilter( 2, "wrl", "x3d" );
+    return _( "VRML and X3D files" ) + AddFileExtListToFilter( { "wrl", "x3d" } );
 }
 
 
 wxString IDF3DFileWildcard()
 {
-    return _( "IDFv3 footprint files" ) + AddFileExtListToFilter( 1, "idf" );
+    return _( "IDFv3 footprint files" ) + AddFileExtListToFilter( { "idf" } );
 }
 
 
 wxString TextFileWildcard()
 {
-    return _( "Text files" ) + AddFileExtListToFilter( 1, "txt" );
+    return _( "Text files" ) + AddFileExtListToFilter( { "txt" } );
 }
 
 
 wxString ModLegacyExportFileWildcard()
 {
-    return _( "Legacy footprint export files" ) + AddFileExtListToFilter( 1, "emp" );
+    return _( "Legacy footprint export files" ) + AddFileExtListToFilter( { "emp" } );
 }
 
 
 wxString ErcFileWildcard()
 {
-    return _( "Electronic rule check file" ) + AddFileExtListToFilter( 1, "erc" );
+    return _( "Electronic rule check file" ) + AddFileExtListToFilter( { "erc" } );
 }
 
 
 wxString SpiceLibraryFileWildcard()
 {
-    return _( "Spice library file" ) + AddFileExtListToFilter( 1, "lib" );
+    return _( "Spice library file" ) + AddFileExtListToFilter( { "lib" } );
 }
 
 
 wxString SpiceNetlistFileWildcard()
 {
-    return _( "SPICE netlist file" ) + AddFileExtListToFilter( 1, "cir" );
+    return _( "SPICE netlist file" ) + AddFileExtListToFilter( { "cir" } );
 }
 
 
 wxString CadstarNetlistFileWildcard()
 {
-    return _( "CadStar netlist file" ) + AddFileExtListToFilter( 1, "frp" );
+    return _( "CadStar netlist file" ) + AddFileExtListToFilter( { "frp" } );
 }
 
 
 wxString EquFileWildcard()
 {
-    return _( "Symbol footprint association files" ) + AddFileExtListToFilter( 1, "equ" );
+    return _( "Symbol footprint association files" ) + AddFileExtListToFilter( { "equ" } );
 }
 
 
 wxString ZipFileWildcard()
 {
-    return _( "Zip file" ) + AddFileExtListToFilter( 1, "zip" );
+    return _( "Zip file" ) + AddFileExtListToFilter( { "zip" } );
 }
 
 
 wxString GencadFileWildcard()
 {
-    return _( "GenCAD 1.4 board files" ) + AddFileExtListToFilter( 1, "cad" );
+    return _( "GenCAD 1.4 board files" ) + AddFileExtListToFilter( { "cad" } );
 }
 
 
 wxString DxfFileWildcard()
 {
-    return _( "DXF Files" ) + AddFileExtListToFilter( 1, "dxf" );
+    return _( "DXF Files" ) + AddFileExtListToFilter( { "dxf" } );
 }
 
 
 wxString GerberJobFileWildcard()
 {
-    return _( "Gerber job file" ) + AddFileExtListToFilter( 1, "gbrjob" );
+    return _( "Gerber job file" ) + AddFileExtListToFilter( { "gbrjob" } );
 }
 
 
 wxString SpecctraDsnFileWildcard()
 {
-    return _( "Specctra DSN file" ) + AddFileExtListToFilter( 1, "dsn" );
+    return _( "Specctra DSN file" ) + AddFileExtListToFilter( { "dsn" } );
 }
 
 
 wxString IpcD356FileWildcard()
 {
-    return _( "IPC-D-356 Test Files" ) + AddFileExtListToFilter( 1, "d356" );
+    return _( "IPC-D-356 Test Files" ) + AddFileExtListToFilter( { "d356" } );
 }
 
 
 wxString WorkbookFileWildcard()
 {
-    return _( "Workbook file" ) + AddFileExtListToFilter( 1, "wbk" );
+    return _( "Workbook file" ) + AddFileExtListToFilter( { "wbk" } );
 }
 
 
 wxString PngFileWildcard()
 {
-    return _( "PNG file" ) + AddFileExtListToFilter( 1, "png" );
+    return _( "PNG file" ) + AddFileExtListToFilter( { "png" } );
 }
 
 
 wxString JpegFileWildcard()
 {
-    return _( "Jpeg file" ) + AddFileExtListToFilter( 2, "jpg", "jpeg" );
+    return _( "Jpeg file" ) + AddFileExtListToFilter( { "jpg", "jpeg" } );
 }
diff --git a/include/wildcards_and_files_ext.h b/include/wildcards_and_files_ext.h
index 182c914f7..f48588a77 100644
--- a/include/wildcards_and_files_ext.h
+++ b/include/wildcards_and_files_ext.h
@@ -34,6 +34,9 @@
 
 #include <wx/wx.h>
 
+#include <string>
+#include <vector>
+
 /**
  * \defgroup file_extensions File Extension Definitions
  *
@@ -47,20 +50,24 @@
  */
 
 /**
- * build the wildcard extension file dialog wildcard filter to add to the base message dialog.
- * for instance, to open .txt files in a file dialog:
+ * Build the wildcard extension file dialog wildcard filter to add to the base message dialog.
+ *
+ * For instance, to open .txt files in a file dialog:
  * the base message is for instance "Text files"
  * the ext list is " (*.txt)|*.txt"
  * and the returned string to add to the base message is " (*.txt)|*.txt"
  * the message to display in the dialog is  "Text files (*.txt)|*.txt"
  *
- * @param aArgCnt is the count of file ext to add to the filter
- * other params are the const char* file ext to add to the filter
+ * This function produces a case-insensitive filter (so .txt, .TXT and .tXT
+ * are all match if you pass "txt" into the function).
+ *
+ * @param aExts is the list of exts to add to the filter. Do not include the
+ * leading dot.
  *
  * @return the appropriate file dialog wildcard filter list.
  */
 
-wxString AddFileExtListToFilter( int aArgCnt, ... );
+wxString AddFileExtListToFilter( const std::vector<std::string>& aExts );
 
 
 // Do NOT use wxString for these.  wxStrings are not thread-safe, even when const.  (For the
diff --git a/qa/common/test_wildcards_and_files_ext.cpp b/qa/common/test_wildcards_and_files_ext.cpp
index 8bff68a83..78e88c59e 100644
--- a/qa/common/test_wildcards_and_files_ext.cpp
+++ b/qa/common/test_wildcards_and_files_ext.cpp
@@ -73,26 +73,17 @@ static constexpr bool should_use_regex_filters()
 /**
  * Check correct handling of filter strings (as used by WX)
  */
-BOOST_AUTO_TEST_CASE( SingleFilter )
+BOOST_AUTO_TEST_CASE( BasicFilter )
 {
-    const auto&       c = ext_wildcard_cases[0];
-    const std::string exp_filter =
-            should_use_regex_filters() ? c.m_re_filter : c.m_filter_case_insenstive;
-
-    const auto resp = AddFileExtListToFilter( 1, "png" );
-
-    BOOST_CHECK_EQUAL( resp, exp_filter );
-}
-
-BOOST_AUTO_TEST_CASE( MultipleFilter )
-{
-    const auto&       c = ext_wildcard_cases[1];
-    const std::string exp_filter =
-            should_use_regex_filters() ? c.m_re_filter : c.m_filter_case_insenstive;
+    for( const auto& c : ext_wildcard_cases )
+    {
+        const std::string exp_filter =
+                should_use_regex_filters() ? c.m_re_filter : c.m_filter_case_insenstive;
 
-    const auto resp = AddFileExtListToFilter( 2, "png", "gif" );
+        const auto resp = AddFileExtListToFilter( c.m_exts );
 
-    BOOST_CHECK_EQUAL( resp, exp_filter );
+        BOOST_CHECK_EQUAL( resp, exp_filter );
+    }
 }
 
 BOOST_AUTO_TEST_SUITE_END()
-- 
2.20.1

From a076f2769f279a2edd7bbe24557956d3f7d3728a Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Fri, 4 Jan 2019 16:01:41 +0000
Subject: [PATCH 1/3] QA: unit tests for file ext filters

This adds a unit test for the recent AddFileExtListToFilter
to demonstrate how to use it and the expected output.

The unit tests are a bit clunky, as the vararg approach cannot
work winth strings passed at runtime.
---
 qa/common/CMakeLists.txt                   |  1 +
 qa/common/test_wildcards_and_files_ext.cpp | 98 ++++++++++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 qa/common/test_wildcards_and_files_ext.cpp

diff --git a/qa/common/CMakeLists.txt b/qa/common/CMakeLists.txt
index 7ea7084f5..724ebc050 100644
--- a/qa/common/CMakeLists.txt
+++ b/qa/common/CMakeLists.txt
@@ -39,6 +39,7 @@ add_executable( qa_common
     test_hotkey_store.cpp
     test_title_block.cpp
     test_utf8.cpp
+    test_wildcards_and_files_ext.cpp
 
     libeval/test_numeric_evaluator.cpp
 
diff --git a/qa/common/test_wildcards_and_files_ext.cpp b/qa/common/test_wildcards_and_files_ext.cpp
new file mode 100644
index 000000000..8bff68a83
--- /dev/null
+++ b/qa/common/test_wildcards_and_files_ext.cpp
@@ -0,0 +1,98 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see CHANGELOG.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 2
+ * 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, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <unit_test_utils/unit_test_utils.h>
+
+#include <wildcards_and_files_ext.h>
+
+
+BOOST_AUTO_TEST_SUITE( WildcardFileExt )
+
+
+/**
+ * Data used to construct a simple test of one or more extensions
+ * and get a filter string for WX dialogs out
+ */
+struct ExtWildcardFilterCase
+{
+    /// The list of exts handled
+    std::vector<std::string> m_exts;
+
+    /// Filter for case-insensitive environments
+    std::string m_filter_case_insenstive;
+
+    /// Filter for regex-capable environments (case insensitive filter in a
+    /// case-sensitive environment)
+    std::string m_re_filter;
+};
+
+const static std::vector<ExtWildcardFilterCase> ext_wildcard_cases = {
+    {
+            { "png" },
+            " ( *.png)|*.png",
+            " ( *.png)|*.[pP][nN][gG]",
+    },
+    {
+            { "png", "gif" },
+            " ( *.png *.gif)|*.png;*.gif",
+            " ( *.png *.gif)|*.[pP][nN][gG];*.[gG][iI][fF]",
+    },
+};
+
+
+static constexpr bool should_use_regex_filters()
+{
+#ifdef __WXGTK__
+    return true;
+#else
+    return false;
+#endif
+}
+
+
+/**
+ * Check correct handling of filter strings (as used by WX)
+ */
+BOOST_AUTO_TEST_CASE( SingleFilter )
+{
+    const auto&       c = ext_wildcard_cases[0];
+    const std::string exp_filter =
+            should_use_regex_filters() ? c.m_re_filter : c.m_filter_case_insenstive;
+
+    const auto resp = AddFileExtListToFilter( 1, "png" );
+
+    BOOST_CHECK_EQUAL( resp, exp_filter );
+}
+
+BOOST_AUTO_TEST_CASE( MultipleFilter )
+{
+    const auto&       c = ext_wildcard_cases[1];
+    const std::string exp_filter =
+            should_use_regex_filters() ? c.m_re_filter : c.m_filter_case_insenstive;
+
+    const auto resp = AddFileExtListToFilter( 2, "png", "gif" );
+
+    BOOST_CHECK_EQUAL( resp, exp_filter );
+}
+
+BOOST_AUTO_TEST_SUITE_END()
-- 
2.20.1


Follow ups