← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Wildcard tests + removed variadic interface

 

Hi Wayne,

It means someone else added a test! :-)

Here is a rebase of these patches.

Cheers,

John

On Wed, Jan 9, 2019 at 1:58 AM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> John,
>
> Would you please rebase your patches?  Patch 0001 no longer applies
> cleaning.  Apparently someone made a change the caused a conflict before
> I could get around to merging them.
>
> Thanks,
>
> Wayne
>
> On 1/8/2019 12:35 PM, John Beard wrote:
> > Hi Seth,
> >
> > Here are updated patches (only #3 is changed).
> >
> > I have still hardcoded the expected results in the test, otherwise
> > it'll be a fairly opaque test of two copy-pasted lines.
> >
> > The windows filter is now " (*.*)|*.*": is that right?
> >
> > Cheers,
> >
> > John
> >
> > On Tue, Jan 8, 2019 at 4:48 PM Seth Hillbrand <seth@xxxxxxxxxxxxx> wrote:
> >>
> >> Am 2019-01-08 11:46, schrieb John Beard:
> >>> Hi Seth,
> >>>
> >>> Good idea - I was just moving what the code currently did, but sounds
> >>> like a sensible fix. I'll amend patch 0003.
> >>>
> >>> Does it otherwise work on Windows?
> >>
> >> Unfortunately, I don't have a Windows setup so we'll need to wait for an
> >> available Windows tester on the patch.
> >>
> >> -S
> >>
> >> _______________________________________________
> >> Mailing list: https://launchpad.net/~kicad-developers
> >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >> Unsubscribe : https://launchpad.net/~kicad-developers
> >> More help   : https://help.launchpad.net/ListHelp
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
From 352d7a199df2c359567d25c48a829c843421893b 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         | 15 +++++++++++++--
 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 | 18 ++++++++++++++++++
 8 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/common/wildcards_and_files_ext.cpp b/common/wildcards_and_files_ext.cpp
index 3f9eb12f0..1274700f8 100644
--- a/common/wildcards_and_files_ext.cpp
+++ b/common/wildcards_and_files_ext.cpp
@@ -68,9 +68,17 @@ static wxString formatWildcardExt( const wxString& aWildcard )
 #endif
 }
 
-
 wxString AddFileExtListToFilter( const std::vector<std::string>& aExts )
 {
+    if( aExts.size() == 0 )
+    {
+        // The "all files" wildcard is different on different systems
+        wxString filter;
+        filter << " (" << wxFileSelectorDefaultWildcardStr << ")|"
+               << wxFileSelectorDefaultWildcardStr;
+        return filter;
+    }
+
     wxString files_filter = " (";
 
     // Add extensions to the info message:
@@ -136,7 +144,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 ac60b8bde..416543a21 100644
--- a/eeschema/dialogs/dialog_netlist.cpp
+++ b/eeschema/dialogs/dialog_netlist.cpp
@@ -632,7 +632,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..6efdd5336 100644
--- a/qa/common/test_wildcards_and_files_ext.cpp
+++ b/qa/common/test_wildcards_and_files_ext.cpp
@@ -86,4 +86,22 @@ BOOST_AUTO_TEST_CASE( BasicFilter )
     }
 }
 
+static constexpr bool should_use_windows_filters()
+{
+#ifdef __WXMSW__
+    return true;
+#else
+    return false;
+#endif
+}
+
+BOOST_AUTO_TEST_CASE( AllFilesFilter )
+{
+    const auto resp = AddFileExtListToFilter( {} );
+
+    const std::string exp_filter = should_use_windows_filters() ? " (*.*)|*.*" : " (*)|*";
+
+    BOOST_CHECK_EQUAL( resp, exp_filter );
+}
+
 BOOST_AUTO_TEST_SUITE_END()
-- 
2.20.1

From 0b5f388a0c4f73d653eb306968712f19fc3938ef 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 106b1cd67e8758dcffcd0bc5c340a3da3d40c335 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                   |  3 +-
 qa/common/test_wildcards_and_files_ext.cpp | 98 ++++++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 qa/common/test_wildcards_and_files_ext.cpp

diff --git a/qa/common/CMakeLists.txt b/qa/common/CMakeLists.txt
index 80aa58e75..ab5bdcd26 100644
--- a/qa/common/CMakeLists.txt
+++ b/qa/common/CMakeLists.txt
@@ -37,10 +37,11 @@ set( common_srcs
     ../../common/observable.cpp
 
     test_color4d.cpp
+    test_format_units.cpp
     test_hotkey_store.cpp
     test_title_block.cpp
     test_utf8.cpp
-    test_format_units.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

References