kicad-developers team mailing list archive

[Oliver Walters <oliver.henry.walters@xxxxxxxxx>]

Important! Patch 0008 (attached) fixes a nullpointer error.

BUT more importantly, it also reintroduces the ability to filter out
libraries using the ':' separator!

Now we are getting somewhere!

On Thu, Nov 16, 2017 at 11:38 PM, Oliver Walters <
oliver.henry.walters@xxxxxxxxx> wrote:

> Wayne,
>
> I have finally found some time to readdress this.
>
> I have reimplemented the filtering using the existing FOOTPRINT_FILTER
> class. I have also reverted behaviour to work only on the currently
> selected footprint library. This removes the requirement to load all the
> libraries when the dialog opens.
>
> I think all requirements are satisfied now. Please find modified patch set
> attached.
>
> Regards,
> Oliver
>
> On Wed, Nov 1, 2017 at 11:21 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
> wrote:
>
>> On 10/31/2017 5:01 PM, Oliver Walters wrote:
>> > How should I proceed here then?
>> >
>> > I would like to see the various libraries being "cached" in the
>> > background, but this is increasing the scope of the work by a large
>> factor.
>> >
>> > One thing I have noticed:
>> >
>> > In eeschema when you launch the component viewer, it (on first run) maps
>> > and caches all the footprint libraries. This can take AGES (especially
>> > on Windows). However on subsequent launches of the component viewer it
>> > appears instantly. It appears to be keeping a static map of the
>> > footprint library data.
>> >
>> > a) Would this be an acceptable approach for the footprint viewer window
>>
>> Sure.  Code reuse is a good thing.  I'm pretty sure the threaded
>> footprint library code is split out from the component chooser so it
>> should be reusable.
>>
>> > b) What happens when the library data changes externally? Does component
>> > viewer need to be reloaded?
>>
>> No, only the library that changed gets reloaded the next time it's
>> accessed.  It is not automatic.  I thought about using wxFileWatcher but
>> that could be a lot of overhead for little net gain.  See the pcb plugin
>> cache() functions.
>>
>> > c) Can we globally perform this caching in a background thread when
>> > KiCad launches? This will hide the large pauses (up to a minute under
>> > Windows) from the user...
>>
>> Yes, this should be done as a project element so that it can be accessed
>> from all of the main windows.  Please keep in mind, this could be a lot
>> of work and given that we are nearing a stable 5 release feature freeze,
>> so if it's not by then it will not make it into the stable 5 release.
>>
>> >
>> > Oliver
>> >
>> > On Tue, Oct 31, 2017 at 11:32 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx
>> > <mailto:stambaughw@xxxxxxxxx>> wrote:
>> >
>> >     On 10/31/2017 1:25 AM, Oliver Walters wrote:
>> >     > Hmm, I had thought that there was a way to load only the *names*
>> of
>> >     > footprints, rather than individually parsing each footprint file.
>> It
>> >     > appears that this is not the case. Any suggestions on how the
>> speed
>> >     > could be improved? Currently I'm reading out all the footprint
>> names in
>> >     > each footprint library and only storing the names (wxString)
>> rather than
>> >     > the MODULE* objects. However, I still have to parse the entire
>> library
>> >     > on load.
>> >     >
>> >     > Ideally, I think it would be good to just read in the names, and
>> then
>> >     > load and display individual MODULE objects on demand.. Is this
>> possible?
>> >
>> >     This is possible (although not implemented) for library types
>> (kicad,
>> >     geda) that use one file per footprint.  You could just read the file
>> >     names from the folder and load the files as required.  If you want
>> to
>> >     search any other properties of the footprint, then you will have to
>> load
>> >     all of the footprints anyway.  I don't know if this would be worth
>> the
>> >     effort.
>> >
>> >     For library types that contain multiple footprints per file (legacy,
>> >     Eagle), this wouldn't make much sense.  Parsing the entire file
>> just to
>> >     pick out the footprint names probably isn't going to save you very
>> much
>> >     time.
>> >
>> >     >
>> >     > On Tue, Oct 31, 2017 at 10:40 AM, Wayne Stambaugh <
>> stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>> >     > <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>>
>> wrote:
>> >     >
>> >     >     On 10/30/2017 5:23 PM, Oliver Walters wrote:
>> >     >     > Thanks for the suggestions on fixing the text. I have that
>> sorted.
>> >     >     >
>> >     >     > I will look into different ways of caching footprint data
>> so it is quicker.
>> >     >     >
>> >     >     > Wayne, I didn't know about FOOTPRINT_FILTER I will switch
>> to using that
>> >     >     > instead (and provide regex search).
>> >     >
>> >     >     Thanks Oliver!
>> >     >
>> >     >     >
>> >     >     > On 31 Oct 2017 06:55, "Seth Hillbrand" <
>> seth.hillbrand@xxxxxxxxx <mailto:seth.hillbrand@xxxxxxxxx>
>> >     <mailto:seth.hillbrand@xxxxxxxxx <mailto:seth.hillbrand@xxxxxxxxx>>
>> >     >     > <mailto:seth.hillbrand@xxxxxxxxx <mailto:
>> seth.hillbrand@xxxxxxxxx>
>> >     <mailto:seth.hillbrand@xxxxxxxxx
>> >     <mailto:seth.hillbrand@xxxxxxxxx>>>> wrote:
>> >     >     >
>> >     >     >     On Mon, Oct 30, 2017 at 11:42 AM, Wayne Stambaugh
>> >     >     >     <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>> >     <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>
>> >     >     <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>> >     <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>>>wrote:
>> >     >     >
>> >     >     >         On 10/30/2017 1:16 PM, Seth Hillbrand wrote:
>> >     >     >         > Oliver, this is neat and very helpful.
>> >     >     >         >
>> >     >     >         > The greyed-out thing is a wx2.8 bug.  You can work
>> >     >     around it by setting
>> >     >     >         > the foreground color when updating the filter like
>> >     this:
>> >     >     >         >
>> >     >     >         >  void FOOTPRINT_VIEWER_FRAME::OnFilterUpdated(
>> >     >     wxCommandEvent& event )
>> >     >     >         >  {
>> >     >     >         > +    // Workaround wx2.8 bug showing greyed color
>> >     >     >         > +    if( m_searchBox->GetValue() !=
>> >     >     m_searchBox->GetDescriptiveText() )
>> >     >     >         > +        m_searchBox->SetForegroundColour(
>> >     >     >         > m_searchBox->GetDefaultAttributes().colFg );
>> >     >     >         > +
>> >     >     >         >      // Filter is non case sensitive
>> >     >     >         >      wxString filter =
>> >     m_searchBox->GetValue().Lower();
>> >     >     >         >
>> >     >     >         > The searchbox handles resetting it to grey on
>> idle()
>> >     >     when the text is empty.
>> >     >     >
>> >     >     >         Don't you mean wx 3.0?  CMake should not even
>> >     generate the
>> >     >     build
>> >     >     >         configuration files without wx 3.0 or greater.
>> >     >     >
>> >     >     >
>> >     >     >     Hmm... This was an issue back in 2.8 that appears to be
>> only
>> >     >     partly
>> >     >     >     fixed.  The workaround I suggest above is functional
>> >     but, for
>> >     >     this,
>> >     >     >     we can also execute a cleaner fix by setting the
>> descriptive
>> >     >     text in
>> >     >     >     the declaration:
>> >     >     >
>> >     >     >     @@ -67,9 +67,10 @@ void
>> >     FOOTPRINT_VIEWER_FRAME::ReCreateHToolbar()
>> >     >     >                                      KiBitmap( module_xpm ),
>> >     >     >                                      _( "Select footprint to
>> >     >     browse" ) );
>> >     >     >
>> >     >     >     -        m_searchBox = new wxSearchCtrl( m_mainToolBar,
>> >     >     >     ID_MODVIEW_SEARCH_TEXT );
>> >     >     >     +        m_searchBox = new wxSearchCtrl( m_mainToolBar,
>> >     >     >     ID_MODVIEW_SEARCH_TEXT,
>> >     >     >     +                _( "Enter filter string" ) );
>> >     >     >              m_searchBox->SetMinSize( wxSize( 250, 30 ) );
>> >     >     >     -        m_searchBox->SetDescriptiveText( _( "Enter
>> filter
>> >     >     string" ) );
>> >     >     >
>> >     >     >
>> >     >     >
>> >     >     >     _______________________________________________
>> >     >     >     Mailing list: https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>
>> >     >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>>
>> >     >     >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> >     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>> >     >     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> >     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>>
>> >     >     >     Unsubscribe : https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>
>> >     >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>>
>> >     >     >     More help   : https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>
>> >     >     <https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>>
>> >     >     >     <https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>
>> >     >     <https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>>>
>> >     >     >
>> >     >     >
>> >     >     >
>> >     >     > _______________________________________________
>> >     >     > Mailing list: https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>
>> >     >     > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> >     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>> >     >     > Unsubscribe : https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>
>> >     >     > More help   : https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>
>> >     >     <https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>>
>> >     >     >
>> >     >
>> >     >
>> >     >     _______________________________________________
>> >     >     Mailing list: https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>
>> >     >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> >     >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>> >     >     Unsubscribe : https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>
>> >     >     <https://launchpad.net/~kicad-developers
>> >     <https://launchpad.net/~kicad-developers>>
>> >     >     More help   : https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>
>> >     >     <https://help.launchpad.net/ListHelp
>> >     <https://help.launchpad.net/ListHelp>>
>> >     >
>> >     >
>> >
>> >
>>
>>
>

From 33cc5c0d094468bc0f019e24c1a0de266f888860 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@xxxxxxxxx>
Date: Fri, 17 Nov 2017 00:00:11 +1100
Subject: [PATCH 8/8] Reimplmented library filtering

- Filtering with the ':' character includes the library name in the filter
- Catching a nullptr
---
 pcbnew/modview_frame.cpp | 62 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/pcbnew/modview_frame.cpp b/pcbnew/modview_frame.cpp
index 3a8452e..283a119 100644
--- a/pcbnew/modview_frame.cpp
+++ b/pcbnew/modview_frame.cpp
@@ -373,19 +373,65 @@ void FOOTPRINT_VIEWER_FRAME::FilterLibs()
         return;
     }
 
-    m_footprintList->Freeze();
-    m_footprintList->Clear();
+    wxString libFilter;
 
-    m_footprintFilter.ClearFilters();
+    /*
+     * If the filter includes the ':' character,
+     * that means that the library nickname should be also filtered!
+     */
+    if( m_filterText.Contains( ":" ) )
+    {
+        wxArrayString split = wxSplit( m_filterText, ':' );
+
+        if( split.Count() > 0 )
+        {
+            libFilter = split.Item( 0 );
+        }
+    }
 
-    m_footprintFilter.FilterByPattern( m_filterText );
+    m_libList->Freeze();
+    m_libList->Clear();
 
-    for( auto& it : m_footprintFilter )
+    for( auto lib : m_libraryNicknames )
     {
-        m_footprintList->Append( it.GetFootprintName() );
+        // Only show libraries that match the library filter (if there is one)
+        if( libFilter.IsEmpty() || lib.Lower().Matches( libFilter ) )
+        {
+            m_libList->Append( lib );
+        }
     }
 
-    m_footprintList->Thaw();
+    m_libList->Thaw();
+
+    // If a previously selected library was filtered out, deselect
+    int index = m_libList->FindString( getCurNickname() );
+
+    if( index != wxNOT_FOUND )
+    {
+        m_libList->SetSelection( index, true );
+    }
+    else
+    {
+        setCurNickname( wxEmptyString );
+        setCurFootprintName( wxEmptyString );
+    }
+
+    if( !getCurNickname().IsEmpty() && m_fpList != nullptr )
+    {
+        m_footprintList->Freeze();
+        m_footprintList->Clear();
+
+        m_footprintFilter.ClearFilters();
+
+        m_footprintFilter.FilterByPattern( m_filterText );
+
+        for( auto& it : m_footprintFilter )
+        {
+            m_footprintList->Append( it.GetFootprintName() );
+        }
+
+        m_footprintList->Thaw();
+    }
 }
 
 
@@ -474,7 +520,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateLibraryList()
 
 void FOOTPRINT_VIEWER_FRAME::ReCreateFootprintList()
 {
-    if( !getCurNickname() )
+    if( getCurNickname().IsEmpty() )
     {
         setCurFootprintName( wxEmptyString );
         return;
-- 
2.7.4