Allow incrementing different parts of strings with modifiers

Primary increment is the right most bit, secondary is the next
rightmost. So you can increment 'A1' to 'A2' or 'B1' with
Shift-Alt-Scroll and Ctrl-Alt-Scroll respectively.

common/CMakeLists.txt

@@ -151,6 +151,7 @@ set( KICOMMON_SRCS
     env_vars.cpp
     exceptions.cpp
     gestfich.cpp
+    increment.cpp
     json_conversions.cpp
     kidialog.cpp
     kiid.cpp
@@ -583,7 +584,6 @@ set( COMMON_SRCS
     grid_tricks.cpp
     hotkey_store.cpp
     hotkeys_basic.cpp
-    increment.cpp
     kiface_base.cpp
     kiway_player.cpp
     lib_table_grid_tricks.cpp

common/array_axis.cpp

@@ -23,6 +23,8 @@
 
 #include <array_axis.h>
 
+#include <increment.h>
+
 
 /**
  * @return False for schemes like 0,1...9,10
@@ -134,26 +136,9 @@ wxString ARRAY_AXIS::GetItemNumber( int n ) const
 {
     wxString        itemNum;
     const wxString& alphabet = GetAlphabet();
-
-    const bool nonUnitColsStartAt0 = schemeNonUnitColsStartAt0( m_type );
-
-    bool firstRound = true;
-    int  radix = alphabet.Length();
+    const bool      nonUnitColsStartAt0 = schemeNonUnitColsStartAt0( m_type );
 
     n = m_offset + m_step * n;
 
-    do
-    {
-        int modN = n % radix;
-
-        if( nonUnitColsStartAt0 && !firstRound )
-            modN--; // Start the "tens/hundreds/etc column" at "Ax", not "Bx"
-
-        itemNum.insert( 0, 1, alphabet[modN] );
-
-        n /= radix;
-        firstRound = false;
-    } while( n );
-
-    return itemNum;
+    return AlphabeticFromIndex( n, alphabet, nonUnitColsStartAt0 );
 }

common/increment.cpp

@@ -25,7 +25,12 @@
 
 #include <wx/wxcrt.h>
 
-bool IncrementString( wxString& name, int aIncrement )
+#include <cmath>
+#include <iostream>
+#include <regex>
+
+
+KICOMMON_API bool IncrementString( wxString& name, int aIncrement )
 {
     if( name.IsEmpty() )
         return true;
@@ -74,4 +79,201 @@ bool IncrementString( wxString& name, int aIncrement )
     }
 
     return false;
-}
+}
+
+
+std::optional<wxString> STRING_INCREMENTER::Increment( const wxString& aStr, int aDelta,
+                                                       size_t aRightIndex ) const
+{
+    if( aStr.IsEmpty() )
+        return std::nullopt;
+
+    wxString                                           remaining = aStr;
+    std::vector<std::pair<wxString, STRING_PART_TYPE>> parts;
+    size_t                                             goodParts = 0;
+
+    // Keep popping chunks off the string until we have what we need
+    while( goodParts < ( aRightIndex + 1 ) && !remaining.IsEmpty() )
+    {
+        static const std::regex integerRegex( R"(\d+$)" );
+        // ABC or abc but not Abc
+        static const std::regex sameCaseAlphabetRegex( R"(([a-z]+|[A-Z]+)$)" );
+        // Skippables - for now anything that isn't a letter or number
+        static const std::regex skipRegex( R"([^a-zA-Z0-9]+$)" );
+
+        std::string remainingStr = remaining.ToStdString();
+        std::smatch match;
+        if( std::regex_search( remainingStr, match, integerRegex ) )
+        {
+            parts.push_back( { match.str(), STRING_PART_TYPE::INTEGER } );
+            remaining = remaining.Left( remaining.Len() - match.str().size() );
+            goodParts++;
+        }
+        else if( std::regex_search( remainingStr, match, sameCaseAlphabetRegex ) )
+        {
+            parts.push_back( { match.str(), STRING_PART_TYPE::ALPHABETIC } );
+            remaining = remaining.Left( remaining.Len() - match.str().size() );
+            goodParts++;
+        }
+        else if( std::regex_search( remainingStr, match, skipRegex ) )
+        {
+            parts.push_back( { match.str(), STRING_PART_TYPE::SKIP } );
+            remaining = remaining.Left( remaining.Len() - match.str().size() );
+        }
+        else
+        {
+            // Out of ideas
+            break;
+        }
+    }
+
+    // Couldn't find the part we wanted
+    if( goodParts < aRightIndex + 1 )
+        return std::nullopt;
+
+    // Increment the part we wanted
+    bool didIncrement = incrementPart( parts.back().first, parts.back().second, aDelta );
+
+    if( !didIncrement )
+        return std::nullopt;
+
+    // Reassemble the string - the left-over part, then parts in reverse
+    wxString result = remaining;
+    for( auto it = parts.rbegin(); it != parts.rend(); ++it )
+    {
+        result << it->first;
+    }
+    return result;
+}
+
+
+static bool containsIOSQXZ( const wxString& aStr )
+{
+    static const wxString iosqxz = "IOSQXZ";
+    for( const wxUniChar& c : aStr )
+    {
+        if( iosqxz.Contains( c ) )
+            return true;
+    }
+    return false;
+}
+
+
+bool STRING_INCREMENTER::incrementPart( wxString& aPart, STRING_PART_TYPE aType, int aDelta ) const
+{
+    switch( aType )
+    {
+    case STRING_PART_TYPE::INTEGER:
+    {
+        long   number = 0;
+        bool   zeroPadded = aPart.StartsWith( '0' );
+        size_t oldLen = aPart.Len();
+
+        if( aPart.ToLong( &number ) )
+        {
+            number += aDelta;
+
+            // Going below zero makes things awkward
+            // and is not usually that useful.
+            if( number < 0 )
+                return false;
+
+            aPart.Printf( "%ld", number );
+
+            // If the number was zero-padded, we need to re-pad it
+            if( zeroPadded )
+            {
+                aPart = wxString( "0", oldLen - aPart.Len() ) + aPart;
+            }
+            return true;
+        }
+
+        break;
+    }
+    case STRING_PART_TYPE::ALPHABETIC:
+    {
+        // Covert to uppercase
+        wxString upper = aPart.Upper();
+        bool     wasUpper = aPart == upper;
+
+        static const wxString alphabetFull = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+        static const wxString alphaNoIOSQXZ = "ABCDEFGHJKLMNPRTUVWY";
+
+        const wxString& alpha =
+                ( m_SkipIOSQXZ & !containsIOSQXZ( aPart ) ) ? alphaNoIOSQXZ : alphabetFull;
+
+        int index = IndexFromAlphabetic( upper, alpha );
+
+        // Something was not in the alphabet
+        if( index == -1 )
+            return false;
+
+        // It's such a big number that we don't want to increment it
+        if( index > m_AlphabeticMaxIndex && m_AlphabeticMaxIndex >= 0 )
+            return false;
+
+        index += aDelta;
+
+        if( index < 0 )
+            return false;
+
+        wxString newStr = AlphabeticFromIndex( index, alpha, true );
+
+        if( !wasUpper )
+            newStr = newStr.Lower();
+
+        aPart = newStr;
+
+        return true;
+    }
+    case STRING_PART_TYPE::SKIP: break;
+    }
+
+    return false;
+}
+
+
+KICOMMON_API int IndexFromAlphabetic( const wxString& aStr, const wxString& aAlphabet )
+{
+    int       index = 0;
+    const int radix = aAlphabet.Length();
+
+    for( size_t i = 0; i < aStr.Len(); i++ )
+    {
+        int alphaIndex = aAlphabet.Find( aStr[i] );
+
+        if( alphaIndex == wxNOT_FOUND )
+            return -1;
+
+        if( i != aStr.Len() - 1 )
+            alphaIndex++;
+
+        index += alphaIndex * pow( radix, aStr.Len() - 1 - i );
+    }
+
+    return index;
+}
+
+
+wxString KICOMMON_API AlphabeticFromIndex( size_t aN, const wxString& aAlphabet,
+                                           bool aZeroBasedNonUnitCols )
+{
+    wxString  itemNum;
+    bool      firstRound = true;
+    const int radix = aAlphabet.Length();
+
+    do
+    {
+        int modN = aN % radix;
+
+        if( aZeroBasedNonUnitCols && !firstRound )
+            modN--; // Start the "tens/hundreds/etc column" at "Ax", not "Bx"
+
+        itemNum.insert( 0, 1, aAlphabet[modN] );
+
+        aN /= radix;
+        firstRound = false;
+    } while( aN );
+
+    return itemNum;
+}

eeschema/tools/sch_edit_tool.cpp

@@ -2984,6 +2984,11 @@ int SCH_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
     if( !allSameType )
         return 0;
 
+    STRING_INCREMENTER incrementer;
+    // In schematics, it's probably less common to be operating
+    // on pin numbers which are uusally IOSQXZ-skippy.
+    incrementer.SetSkipIOSQXZ( false );
+
     SCH_COMMIT commit( m_frame );
 
     for( EDA_ITEM* item : selection )
@@ -2995,16 +3000,14 @@ int SCH_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
         case SCH_HIER_LABEL_T:
         case SCH_TEXT_T:
         {
-            // Only support the first index for now
-            if( incParam.Index == 0 )
+            SCH_TEXT& label = static_cast<SCH_TEXT&>( *item );
+
+            std::optional<wxString> newLabel =
+                    incrementer.Increment( label.GetText(), incParam.Delta, incParam.Index );
+            if( newLabel )
             {
-                SCH_TEXT& label = static_cast<SCH_TEXT&>( *item );
-
-                wxString nextLabel = label.GetText();
-                IncrementString( nextLabel, incParam.Delta );
-
                 commit.Modify( &label, m_frame->GetScreen() );
-                label.SetText( nextLabel );
+                label.SetText( *newLabel );
             }
             break;
         }

eeschema/tools/symbol_editor_edit_tool.cpp

@@ -1120,6 +1120,9 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
 
     const VECTOR2I mousePosition = getViewControls()->GetMousePosition();
 
+    STRING_INCREMENTER incrementer;
+    incrementer.SetSkipIOSQXZ( true );
+
     SCH_COMMIT commit( m_frame );
 
     for( EDA_ITEM* item : selection )
@@ -1128,51 +1131,53 @@ int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
         {
         case SCH_PIN_T:
         {
-            // Primary increment: name or number of the pin
-            if( incParam.Index == 0 )
-            {
-                SCH_PIN&          pin = static_cast<SCH_PIN&>( *item );
-                PIN_LAYOUT_CACHE& layout = pin.GetLayoutCache();
+            SCH_PIN&          pin = static_cast<SCH_PIN&>( *item );
+            PIN_LAYOUT_CACHE& layout = pin.GetLayoutCache();
 
-                bool      found = false;
-                OPT_BOX2I bbox = layout.GetPinNumberBBox();
+            bool      found = false;
+            OPT_BOX2I bbox = layout.GetPinNumberBBox();
+
+            if( bbox && bbox->Contains( mousePosition ) )
+            {
+                std::optional<wxString> nextNumber =
+                        incrementer.Increment( pin.GetNumber(), incParam.Delta, incParam.Index );
+                if( nextNumber )
+                {
+                    commit.Modify( &pin );
+                    pin.SetNumber( *nextNumber );
+                }
+                found = true;
+            }
+
+            if( !found )
+            {
+                bbox = layout.GetPinNameBBox();
 
                 if( bbox && bbox->Contains( mousePosition ) )
                 {
-                    wxString nextNumber = pin.GetNumber();
-                    IncrementString( nextNumber, incParam.Delta );
-
-                    commit.Modify( &pin );
-                    pin.SetNumber( nextNumber );
-                    found = true;
-                }
-
-                if( !found )
-                {
-                    bbox = layout.GetPinNameBBox();
-
-                    if( bbox && bbox->Contains( mousePosition ) )
+                    std::optional<wxString> nextName =
+                            incrementer.Increment( pin.GetName(), incParam.Delta, incParam.Index );
+                    if( nextName )
                     {
-                        wxString nextName = pin.GetName();
-                        IncrementString( nextName, incParam.Delta );
-
                         commit.Modify( &pin );
-                        pin.SetName( nextName );
-                        found = true;
+                        pin.SetName( *nextName );
                     }
+                    found = true;
                 }
             }
             break;
         }
         case SCH_TEXT_T:
         {
-            SCH_TEXT& text = static_cast<SCH_TEXT&>( *item );
+            SCH_TEXT& label = static_cast<SCH_TEXT&>( *item );
 
-            wxString nextText = text.GetText();
-            IncrementString( nextText, incParam.Delta );
-
-            commit.Modify( &text );
-            text.SetText( nextText );
+            std::optional<wxString> newLabel =
+                    incrementer.Increment( label.GetText(), incParam.Delta, incParam.Index );
+            if( newLabel )
+            {
+                commit.Modify( &label, m_frame->GetScreen() );
+                label.SetText( *newLabel );
+            }
             break;
         }
         default:

include/increment.h

@@ -23,6 +23,8 @@
 
 #pragma once
 
+#include <optional>
+
 #include <wx/string.h>
 
 #include <kicommon.h>
@@ -30,4 +32,75 @@
 /**
  * Generic string incrementer.
  */
-bool IncrementString( wxString& aStr, int aDelta );
+KICOMMON_API bool IncrementString( wxString& aStr, int aDelta );
+
+
+/**
+ * Heuristically increment a string's n'th part from the right.
+ *
+ * For example: incrementing the 0th part of A1 -> A2
+ *                               1st part of A1 -> B1
+ *
+ * This is a bit subjective as to what represents suitable
+ * "incrementable" parts, but it tries to be smart about it.
+ */
+class KICOMMON_API STRING_INCREMENTER
+{
+public:
+    /**
+     * If a alphabetic part is found, skip the letters I, O, S, Q, X, Z.
+     * (if one is already there, increment it anyway).
+     */
+    void SetSkipIOSQXZ( bool aSkip ) { m_SkipIOSQXZ = aSkip; }
+
+    /**
+     * Set the maximum index for alphabetic parts.
+     *
+     * This means that if the index is greater than this, it will be treated
+     * as un-incrementable. This is to avoid incrementing things like "TX" or
+     * "CAN", which would be indexes of hundreds (unlikely to be a BGA row prefix,
+     * for example).
+     *
+     * Setting < 0 disables the check (no limit)
+     */
+    void SetAlphabeticMaxIndex( int aMaxIndex ) { m_AlphabeticMaxIndex = aMaxIndex; }
+
+    /**
+     * Increment the n-th part from the right of the given string.
+     */
+    std::optional<wxString> Increment( const wxString& aStr, int aDelta, size_t aRightIndex ) const;
+
+private:
+    enum class STRING_PART_TYPE
+    {
+        ALPHABETIC,
+        INTEGER,
+        SKIP,
+    };
+
+    bool incrementPart( wxString& aPart, STRING_PART_TYPE aType, int aDelta ) const;
+
+    bool m_SkipIOSQXZ = true;
+    int  m_AlphabeticMaxIndex = 50;
+};
+
+/**
+ * Attempt to convert a string to an integer, assuming it is an alphabetic
+ * string like "A", "B", ... "Z", "AA", "AB", ... "ZZ", "AAA", ... in some
+ * alphabet.
+ *
+ * @return The value of the string, or -1 if a character is
+ *         not in the alphabet.
+ */
+KICOMMON_API int IndexFromAlphabetic( const wxString& aStr, const wxString& aAlphabet );
+
+/**
+ * Get an alphabetic string like A, B, ... Z, AA, AB, ... ZZ, AAA, ...
+ *
+ * @param aIndex    The index to convert.
+ * @param aAlphabet The alphabet to use.
+ * @param aZeroBasedNonUnitCols If true, cols other than the right most use the 0'th entry
+ *                              (e.g. Z -> AA, not BA, but 9 -> 10, not 00).
+ */
+KICOMMON_API wxString AlphabeticFromIndex( size_t aN, const wxString& aAlphabet,
+                                           bool aZeroBasedNonUnitCols );

pcbnew/tools/edit_tool.cpp

@@ -2982,6 +2982,9 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
 
     const ACTIONS::INCREMENT incParam = aEvent.Parameter<ACTIONS::INCREMENT>();
 
+    STRING_INCREMENTER incrementer;
+    incrementer.SetSkipIOSQXZ( true );
+
     BOARD_COMMIT commit( this );
 
     for( EDA_ITEM* item : selection )
@@ -2999,14 +3002,14 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
             if( !pad.CanHaveNumber() )
                 continue;
 
-            // Increment only handled for pad numbers
-            if( incParam.Index == PCB_ACTIONS::PAD_INCREMENT::NUMBER )
-            {
-                wxString padNumber = pad.GetNumber();
-                IncrementString( padNumber, incParam.Delta );
+            // Increment on the pad numbers
+            std::optional<wxString> newNumber =
+                    incrementer.Increment( pad.GetNumber(), incParam.Delta, incParam.Index );
 
+            if( newNumber )
+            {
                 commit.Modify( &pad );
-                pad.SetNumber( padNumber );
+                pad.SetNumber( *newNumber );
             }
             break;
         }
@@ -3014,11 +3017,14 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent )
         {
             PCB_TEXT& text = static_cast<PCB_TEXT&>( *item );
 
-            wxString content = text.GetText();
-            IncrementString( content, incParam.Delta );
+            std::optional<wxString> newText =
+                    incrementer.Increment( text.GetText(), incParam.Delta, incParam.Index );
 
-            commit.Modify( &text );
-            text.SetText( content );
+            if( newText )
+            {
+                commit.Modify( &text );
+                text.SetText( *newText );
+            }
         }
         default:
         {

pcbnew/tools/pcb_actions.h

@@ -588,11 +588,6 @@ public:
 
     static TOOL_ACTION repeatLayout;
     static TOOL_ACTION generatePlacementRuleAreas;
-
-    enum PAD_INCREMENT
-    {
-        NUMBER = 0,
-    };
 };
 
 class PCB_EVENTS

qa/tests/common/CMakeLists.txt

@@ -38,6 +38,7 @@ set( QA_COMMON_SRCS
     test_eda_shape.cpp
     test_eda_text.cpp
     test_embedded_file_compress.cpp
+    test_increment.cpp
     test_lib_table.cpp
     test_markup_parser.cpp
     test_kicad_string.cpp

qa/tests/common/test_increment.cpp

@@ -0,0 +1,129 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2024 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 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 <qa_utils/wx_utils/unit_test_utils.h>
+
+#include <increment.h>
+
+
+/**
+ * Declares a struct as the Boost test fixture.
+ */
+BOOST_AUTO_TEST_SUITE( StringIncrement )
+
+
+struct INCREMENT_TEST_CASE
+{
+    wxString input;
+    int      delta;
+    size_t   part;
+    wxString expected;
+};
+
+
+/**
+ * Check formatting the point
+ */
+BOOST_AUTO_TEST_CASE( BasicCase )
+{
+    const std::vector<INCREMENT_TEST_CASE> cases{
+        // Null
+        { "", 1, 0, "nullopt" },
+        { "", 1, 1, "nullopt" },
+        { "", -1, 1, "nullopt" },
+        // Up
+        { "1", 1, 0, "2" },
+        { "1", 9, 0, "10" },
+        // Down
+        { "2", -1, 0, "1" },
+        { "10", -1, 0, "9" },
+        // Down from 0
+        { "0", -1, 0, "nullopt" },
+        // Ran out of a parts
+        { "1", 1, 1, "nullopt" },
+        // Leading zeros preserved
+        { "01", 1, 0, "02" },
+        // Alpha
+        { "A", 1, 0, "B" },
+        { "E", -1, 0, "D" },
+        // Skip I
+        { "H", 1, 0, "J" },
+        { "J", -1, 0, "H" },
+        // But I works if it's there
+        { "I", 1, 0, "J" },
+        { "I", -1, 0, "H" },
+        // Alpha wrap
+        { "Z", 1, 0, "AA" },
+        // Reject huge alphabetic value
+        { "ABB", 1, 0, "nullopt" },
+        // Dashes skipped
+        { "A-1", 1, 0, "A-2" },
+        { "A-1", 1, 1, "B-1" },
+    };
+
+    STRING_INCREMENTER incrementer;
+    incrementer.SetSkipIOSQXZ( true );
+
+    for( const auto& c : cases )
+    {
+        BOOST_TEST_INFO( "Input: " << c.input << " Delta: " << c.delta << " Part: " << c.part );
+        wxString result = incrementer.Increment( c.input, c.delta, c.part ).value_or( "nullopt" );
+        BOOST_CHECK_EQUAL( result, c.expected );
+    }
+}
+
+
+struct ALPHABETIC_TEST_CASE
+{
+    wxString        input;
+    const wxString& alphabet;
+    int             expected;
+};
+
+
+BOOST_AUTO_TEST_CASE( AlphabeticIndexes )
+{
+    const wxString alphabet = "ABCDEFGHJKLMNPRTUVWY";
+
+    const std::vector<ALPHABETIC_TEST_CASE> cases{ {
+            { "A", alphabet, 0 },
+            { "B", alphabet, 1 },
+            { "Y", alphabet, 19 },
+            { "AA", alphabet, 20 },
+            { "AY", alphabet, 39 },
+    } };
+
+    for( const auto& c : cases )
+    {
+        BOOST_TEST_INFO( "Input: " << c.input << " <-> " << c.expected );
+
+        const int fromString = IndexFromAlphabetic( c.input, c.alphabet );
+        BOOST_CHECK_EQUAL( fromString, c.expected );
+
+        const wxString fromIndex = AlphabeticFromIndex( c.expected, c.alphabet, true );
+        BOOST_CHECK_EQUAL( fromIndex, c.input );
+    }
+}
+
+
+BOOST_AUTO_TEST_SUITE_END()