Retire the lib-tree scoring algorithm.

It appears to cause more confusion than help.

Fixes https://gitlab.com/kicad/code/kicad/issues/13905

common/dialogs/dialog_assign_netclass.cpp

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2022-2023 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
@@ -110,10 +110,7 @@ void DIALOG_ASSIGN_NETCLASS::onPatternText( wxCommandEvent& aEvent )
 
             for( const wxString& net : m_netCandidates )
             {
-                int matches;
+                if( matcher.StartsWith( net ) )
-                int offset;
-
-                if( matcher.Find( net, matches, offset ) && offset == 0 )
                 {
                     m_matchingNets->Report( net );
                     matchingNetNames.push_back( net );

common/dialogs/panel_setup_netclasses.cpp

@@ -707,10 +707,7 @@ void PANEL_SETUP_NETCLASSES::OnUpdateUI( wxUpdateUIEvent& event )
 
             for( const wxString& net : m_netNames )
             {
-                int matches;
+                if( matcher.StartsWith( net ) )
-                int offset;
-
-                if( matcher.Find( net, matches, offset ) && offset == 0 )
                     m_matchingNets->Report( net );
             }
         }

common/eda_pattern_match.cpp

@@ -399,25 +399,27 @@ EDA_COMBINED_MATCHER::EDA_COMBINED_MATCHER( const wxString& aPattern,
 }
 
 
-bool EDA_COMBINED_MATCHER::Find( const wxString& aTerm, int& aMatchersTriggered, int& aPosition )
+bool EDA_COMBINED_MATCHER::Find( const wxString& aTerm )
 {
-    aPosition = EDA_PATTERN_NOT_FOUND;
-    aMatchersTriggered = 0;
-
     for( const std::unique_ptr<EDA_PATTERN_MATCH>& matcher : m_matchers )
     {
-        EDA_PATTERN_MATCH::FIND_RESULT local_find = matcher->Find( aTerm );
+        if( matcher->Find( aTerm ).start >= 0 )
-
+            return true;
-        if( local_find )
-        {
-            aMatchersTriggered += 1;
-
-            if( local_find.start < aPosition || aPosition == EDA_PATTERN_NOT_FOUND )
-                aPosition = local_find.start;
-        }
     }
 
-    return aPosition != EDA_PATTERN_NOT_FOUND;
+    return false;
+}
+
+
+bool EDA_COMBINED_MATCHER::StartsWith( const wxString& aTerm )
+{
+    for( const std::unique_ptr<EDA_PATTERN_MATCH>& matcher : m_matchers )
+    {
+        if( matcher->Find( aTerm ).start == 0 )
+            return true;
+    }
+
+    return false;
 }
 
 

common/footprint_filter.cpp

@@ -86,12 +86,11 @@ void FOOTPRINT_FILTER_IT::increment()
                                                    candidate.GetLibNickname(),
                                                    candidate.GetFootprintName(),
                                                    candidate.GetSearchText() );
-            int      matches, position;
+            bool exclude = false;
-            bool     exclude = false;
 
             for( std::unique_ptr<EDA_COMBINED_MATCHER>& matcher : m_filter->m_pattern_filters )
             {
-                if( !matcher->Find( searchStr.Lower(), matches, position ) )
+                if( !matcher->Find( searchStr.Lower() ) )
                 {
                     exclude = true;
                     break;

common/lib_tree_model.cpp

@@ -3,7 +3,7 @@
  *lib_tree_model
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
- * Copyright (C) 2014-2019 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2014-2023 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 as published by the
@@ -24,32 +24,16 @@
 #include <algorithm>
 #include <eda_pattern_match.h>
 #include <lib_tree_item.h>
-#include <utility>
 #include <pgm_base.h>
 #include <string_utils.h>
 
-// Each node gets this lowest score initially, without any matches applied.
+// Each node gets this lowest score initially, without any matches applied.  Matches will then
-// Matches will then increase this score depending on match quality.  This way,
+// increase this score.  This way, an empty search string will result in all components being
-// an empty search string will result in all components being displayed as they
+// displayed as they have the minimum score. However, in that case, we avoid expanding all the
-// have the minimum score. However, in that case, we avoid expanding all the
 // nodes asd the result is very unspecific.
 static const unsigned kLowestDefaultScore = 1;
 
 
-// Creates a score depending on the position of a string match. If the position
-// is 0 (= prefix match), this returns the maximum score. This degrades until
-// pos == max, which returns a score of 0; Evertyhing else beyond that is just
-// 0. Only values >= 0 allowed for position and max.
-//
-// @param aPosition is the position a string has been found in a substring.
-// @param aMaximum is the maximum score this function returns.
-// @return position dependent score.
-static int matchPosScore(int aPosition, int aMaximum)
-{
-    return ( aPosition < aMaximum ) ? aMaximum - aPosition : 0;
-}
-
-
 void LIB_TREE_NODE::ResetScore()
 {
     for( std::unique_ptr<LIB_TREE_NODE>& child: m_Children )
@@ -259,46 +243,14 @@ void LIB_TREE_NODE_LIB_ID::UpdateScore( EDA_COMBINED_MATCHER& aMatcher, const wx
         return;
     }
 
-    // Keywords and description we only count if the match string is at
+    // Keyword and description matches only count if they're at least two chars long.  This
-    // least two characters long. That avoids spurious, low quality
+    // avoids spurious, low quality matches.  (Most abbreviations are at least 3 chars.)
-    // matches. Most abbreviations are at three characters long.
+    if( aMatcher.Find( m_MatchName ) )
-    int found_pos = EDA_PATTERN_NOT_FOUND;
+        m_Score++;
-    int matchers_fired = 0;
+    else if( aMatcher.GetPattern().length() >= 2 && aMatcher.Find( m_SearchText ) )
-
+        m_Score++;
-    if( aMatcher.GetPattern() == m_MatchName )
-    {
-        m_Score += 1000;  // exact match. High score :)
-    }
-    else if( aMatcher.Find( m_MatchName, matchers_fired, found_pos ) )
-    {
-        // Substring match. The earlier in the string the better.
-        m_Score += matchPosScore( found_pos, 20 ) + 20;
-    }
-    else if( aMatcher.Find( m_Parent->m_MatchName, matchers_fired, found_pos ) )
-    {
-        m_Score += 19;   // parent name matches.         score += 19
-    }
-    else if( aMatcher.Find( m_SearchText, matchers_fired, found_pos ) )
-    {
-        // If we have a very short search term (like one or two letters),
-        // we don't want to accumulate scores if they just happen to be in
-        // keywords or description as almost any one or two-letter
-        // combination shows up in there.
-        if( aMatcher.GetPattern().length() >= 2 )
-        {
-            // For longer terms, we add scores 1..18 for positional match
-            // (higher in the front, where the keywords are).
-            m_Score += matchPosScore( found_pos, 17 ) + 1;
-        }
-    }
     else
-    {
-        // No match. That's it for this item.
         m_Score = 0;
-    }
-
-    // More matchers = better match
-    m_Score += 2 * matchers_fired;
 }
 
 
@@ -340,27 +292,10 @@ void LIB_TREE_NODE_LIB::UpdateScore( EDA_COMBINED_MATCHER& aMatcher, const wxStr
     {
         // No children; we are a leaf.
 
-        if( !aLib.IsEmpty() )
+        if( !aLib.IsEmpty() && m_MatchName == aLib )
-        {
+            m_Score++;
-            m_Score = m_MatchName == aLib ? 1000 : 0;
+        else if( aMatcher.Find( m_MatchName ) )
-            return;
+            m_Score++;
-        }
-
-        int found_pos = EDA_PATTERN_NOT_FOUND;
-        int matchers_fired = 0;
-
-        if( aMatcher.GetPattern() == m_MatchName )
-        {
-            m_Score += 1000;  // exact match. High score :)
-        }
-        else if( aMatcher.Find( m_MatchName, matchers_fired, found_pos ) )
-        {
-            // Substring match. The earlier in the string the better.
-            m_Score += matchPosScore( found_pos, 20 ) + 20;
-        }
-
-        // More matchers = better match
-        m_Score += 2 * matchers_fired;
     }
 }
 

common/lib_tree_model_adapter.cpp

@@ -3,7 +3,7 @@
  *
  * Copyright (C) 2017 Chris Pavlina <pavlina.chris@gmail.com>
  * Copyright (C) 2014 Henner Zeller <h.zeller@acm.org>
- * Copyright (C) 2014-2022 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2014-2023 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 as published by the
@@ -213,17 +213,11 @@ void LIB_TREE_MODEL_ADAPTER::UpdateSearchString( const wxString& aSearch, bool a
         Thaw();
     }
 
-    LIB_TREE_NODE* bestMatch = ShowResults();
+    const LIB_TREE_NODE* firstMatch = ShowResults();
 
-    if( !bestMatch )
+    if( firstMatch )
-        bestMatch = ShowPreselect();
-
-    if( !bestMatch )
-        bestMatch = ShowSingleLibrary();
-
-    if( bestMatch )
     {
-        wxDataViewItem item = wxDataViewItem( bestMatch );
+        wxDataViewItem item = ToItem( firstMatch );
         m_widget->Select( item );
 
         // Make sure the *parent* item is visible. The selected item is the
@@ -619,93 +613,83 @@ bool LIB_TREE_MODEL_ADAPTER::GetAttr( const wxDataViewItem&   aItem,
 }
 
 
-void LIB_TREE_MODEL_ADAPTER::Find( LIB_TREE_NODE& aNode,
+void recursiveDescent( LIB_TREE_NODE& aNode, const std::function<bool( const LIB_TREE_NODE* )>& f )
-                                            std::function<bool( const LIB_TREE_NODE* )> aFunc,
-                                            LIB_TREE_NODE** aHighScore )
 {
     for( std::unique_ptr<LIB_TREE_NODE>& node: aNode.m_Children )
     {
-        if( aFunc( &*node ) )
+        if( !f( node.get() ) )
-        {
+            break;
-            if( !(*aHighScore) || node->m_Score > (*aHighScore)->m_Score )
-                (*aHighScore) = &*node;
-        }
 
-        Find( *node, aFunc, aHighScore );
+        recursiveDescent( *node, f );
     }
 }
 
 
-LIB_TREE_NODE* LIB_TREE_MODEL_ADAPTER::ShowResults()
+const LIB_TREE_NODE* LIB_TREE_MODEL_ADAPTER::ShowResults()
 {
-    LIB_TREE_NODE* highScore = nullptr;
+    const LIB_TREE_NODE* firstMatch = nullptr;
 
-    Find( m_tree,
+    // Expand parents of leaf nodes with some level of matching
-                   []( LIB_TREE_NODE const* n )
+    recursiveDescent( m_tree,
-                   {
+            [&]( const LIB_TREE_NODE* n )
-                       // return leaf nodes with some level of matching
-                       return n->m_Type == LIB_TREE_NODE::TYPE::LIBID && n->m_Score > 1;
-                   },
-                   &highScore );
-
-    if( highScore)
-    {
-        wxDataViewItem item = wxDataViewItem( highScore );
-        m_widget->ExpandAncestors( item );
-    }
-
-    return highScore;
-}
-
-
-LIB_TREE_NODE* LIB_TREE_MODEL_ADAPTER::ShowPreselect()
-{
-    LIB_TREE_NODE* highScore = nullptr;
-
-    if( !m_preselect_lib_id.IsValid() )
-        return highScore;
-
-    Find( m_tree,
-            [&]( LIB_TREE_NODE const* n )
             {
-                if( n->m_Type == LIB_TREE_NODE::LIBID && ( n->m_Children.empty() ||
+                if( n->m_Type == LIB_TREE_NODE::TYPE::LIBID && n->m_Score > 1 )
-                                                           !m_preselect_unit ) )
+                {
-                    return m_preselect_lib_id == n->m_LibId;
+                    if( !firstMatch )
-                else if( n->m_Type == LIB_TREE_NODE::UNIT && m_preselect_unit )
+                        firstMatch = n;
-                    return m_preselect_lib_id == n->m_Parent->m_LibId &&
-                            m_preselect_unit == n->m_Unit;
-                else
-                    return false;
-            },
-            &highScore );
 
-    if( highScore)
+                    m_widget->ExpandAncestors( ToItem( n ) );
+                }
+
+                return true;    // keep going to expand ancestors of all found items
+            } );
+
+    // If no matches, find and show the preselect node
+    if( !firstMatch && m_preselect_lib_id.IsValid() )
     {
-        wxDataViewItem item = wxDataViewItem( highScore );
+        recursiveDescent( m_tree,
-        m_widget->ExpandAncestors( item );
+                [&]( const LIB_TREE_NODE* n )
+                {
+                    if( n->m_Type == LIB_TREE_NODE::LIBID
+                              && ( n->m_Children.empty() || !m_preselect_unit )
+                              && m_preselect_lib_id == n->m_LibId )
+                    {
+                        firstMatch = n;
+                        m_widget->ExpandAncestors( ToItem( n ) );
+                        return false;
+                    }
+                    else if( n->m_Type == LIB_TREE_NODE::UNIT
+                              && ( m_preselect_unit && m_preselect_unit == n->m_Unit )
+                              && m_preselect_lib_id == n->m_Parent->m_LibId )
+                    {
+                        firstMatch = n;
+                        m_widget->ExpandAncestors( ToItem( n ) );
+                        return false;
+                    }
+
+                    return true;
+                } );
     }
 
-    return highScore;
+    // If still no matches expand a single library if there is only one
+    if( !firstMatch )
+    {
+        recursiveDescent( m_tree,
+                [&]( const LIB_TREE_NODE* n )
+                {
+                    if( n->m_Type == LIB_TREE_NODE::TYPE::LIBID
+                            && n->m_Parent->m_Parent->m_Children.size() == 1 )
+                    {
+                        firstMatch = n;
+                        m_widget->ExpandAncestors( ToItem( n ) );
+                        return false;
+                    }
+
+                    return true;
+                } );
+    }
+
+    return firstMatch;
 }
 
 
-LIB_TREE_NODE* LIB_TREE_MODEL_ADAPTER::ShowSingleLibrary()
-{
-    LIB_TREE_NODE* highScore = nullptr;
-
-    Find( m_tree,
-                   []( LIB_TREE_NODE const* n )
-                   {
-                       return n->m_Type == LIB_TREE_NODE::TYPE::LIBID &&
-                              n->m_Parent->m_Parent->m_Children.size() == 1;
-                   },
-                   &highScore );
-
-    if( highScore)
-    {
-        wxDataViewItem item = wxDataViewItem( highScore );
-        m_widget->ExpandAncestors( item );
-    }
-
-    return highScore;
-}

common/project/net_settings.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2020 CERN
- * Copyright (C) 2021-2022 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2021-2023 KiCad Developers, see AUTHORS.txt for contributors.
  * @author Jon Evans <jon@craftyjon.com>
  *
  * This program is free software: you can redistribute it and/or modify it
@@ -398,10 +398,7 @@ std::shared_ptr<NETCLASS> NET_SETTINGS::GetEffectiveNetClass( const wxString& aN
 
     for( const auto& [ matcher, netclassName ] :  m_NetClassPatternAssignments )
     {
-        int matches;
+        if( matcher->StartsWith( aNetName ) )
-        int offset;
-
-        if( matcher->Find( aNetName, matches, offset ) && offset == 0 )
         {
             m_NetClassPatternAssignmentCache[ aNetName ] = netclassName;
             return getNetclass( netclassName );

eeschema/sim/simulator_frame.cpp

@@ -796,10 +796,7 @@ void SIMULATOR_FRAME::rebuildSignalsGrid( wxString aFilter )
 
     for( const wxString& signal : m_signals )
     {
-        int matches;
+        if( matcher.StartsWith( signal ) )
-        int offset;
-
-        if( matcher.Find( signal, matches, offset ) && offset == 0 )
         {
             int      traceType = SPT_UNKNOWN;
             wxString vectorName = vectorNameFromSignalName( signal, &traceType );

eeschema/symbol_viewer_frame.cpp

@@ -719,11 +719,10 @@ bool SYMBOL_VIEWER_FRAME::ReCreateLibList()
         {
             const wxString       term = tokenizer.GetNextToken().Lower();
             EDA_COMBINED_MATCHER matcher( term, CTX_LIBITEM );
-            int                  matches, position;
 
             for( const wxString& lib : libs )
             {
-                if( matcher.Find( lib.Lower(), matches, position ) )
+                if( matcher.Find( lib.Lower() ) )
                     process( lib );
             }
         }
@@ -796,12 +795,11 @@ bool SYMBOL_VIEWER_FRAME::ReCreateSymbolList()
         {
             const wxString       term = tokenizer.GetNextToken().Lower();
             EDA_COMBINED_MATCHER matcher( term, CTX_LIBITEM );
-            int                  matches, position;
 
             for( LIB_SYMBOL* symbol : symbols )
             {
                 wxString search = symbol->GetName() + wxS( " " ) + symbol->GetSearchText();
-                bool     matched = matcher.Find( search.Lower(), matches, position );
+                bool     matched = matcher.Find( search.Lower() );
 
                 if( !matched && term.IsNumber() )
                     matched = ( wxAtoi( term ) == (int)symbol->GetPinCount() );

include/eda_pattern_match.h

@@ -197,7 +197,9 @@ public:
      *
      * @return true if any matchers found the term
      */
-    bool Find( const wxString& aTerm, int& aMatchersTriggered, int& aPosition );
+    bool Find( const wxString& aTerm );
+
+    bool StartsWith( const wxString& aTerm );
 
     const wxString& GetPattern() const;
 

include/lib_tree_model_adapter.h

@@ -391,26 +391,14 @@ protected:
 
 private:
     /**
-     * Find any results worth highlighting and expand them, according to given criteria
+     * Find any results worth highlighting and expand them.
-     * The highest-scoring node is written to aHighScore
      */
-    void Find( LIB_TREE_NODE& aNode, std::function<bool( const LIB_TREE_NODE* )> aFunc,
+    void Find( LIB_TREE_NODE& aNode, std::function<bool( const LIB_TREE_NODE* )> aFunc );
-               LIB_TREE_NODE** aHighScore );
 
     /**
      * Find and expand successful search results.  Return the best match (if any).
      */
-    LIB_TREE_NODE* ShowResults();
+    const LIB_TREE_NODE* ShowResults();
-
-    /**
-     * Find and expand preselected node.  Return the best match (if any).
-     */
-    LIB_TREE_NODE* ShowPreselect();
-
-    /**
-     * Find and expand a library if there is only one.  Return the best match (if any).
-     */
-    LIB_TREE_NODE* ShowSingleLibrary();
 
     wxDataViewColumn* doAddColumn( const wxString& aHeader, bool aTranslate = true );
 

pcbnew/footprint_viewer_frame.cpp

@@ -493,11 +493,10 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateLibraryList()
         {
             const wxString       term = tokenizer.GetNextToken().Lower();
             EDA_COMBINED_MATCHER matcher( term, CTX_LIBITEM );
-            int                  matches, position;
 
             for( const wxString& nickname : nicknames )
             {
-                if( matcher.Find( nickname.Lower(), matches, position ) )
+                if( matcher.Find( nickname.Lower() ) )
                     process( nickname );
             }
         }
@@ -570,12 +569,11 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateFootprintList()
         {
             const wxString       term = tokenizer.GetNextToken().Lower();
             EDA_COMBINED_MATCHER matcher( term, CTX_LIBITEM );
-            int                  matches, position;
 
             for( const std::unique_ptr<FOOTPRINT_INFO>& footprint : fp_info_list->GetList() )
             {
                 wxString search = footprint->GetFootprintName() + wxS( " " ) + footprint->GetSearchText();
-                bool     matched = matcher.Find( search.Lower(), matches, position );
+                bool     matched = matcher.Find( search.Lower() );
 
                 if( !matched && term.IsNumber() )
                     matched = ( wxAtoi( term ) == (int)footprint->GetPadCount() );