Switch to string_view in tool action construction

The TOOL_ACTION_ARGS construction method is supposed to be fairly light
weight, so instead of creating/destroying strings all the time, pass
string_views to the actual string data instead.

wx doesn't currently support string_view -> wxString implicitly
currently, so we have to do the conversion ourself.

common/tool/tool_action.cpp

@@ -30,7 +30,9 @@
 #include <algorithm>
 #include <bitmaps.h>
 #include <hotkeys_basic.h>
-#include <wx/stringimpl.h>
+
+#include <core/wx_stl_compat.h>
+#include <wx/string.h>
 #include <wx/translation.h>
 
 TOOL_ACTION::TOOL_ACTION( const std::string& aName, TOOL_ACTION_SCOPE aScope,
@@ -69,8 +71,8 @@ TOOL_ACTION::TOOL_ACTION( const TOOL_ACTION_ARGS& aArgs ) :
         m_defaultHotKey( aArgs.m_defaultHotKey.value_or( 0 ) ),
         m_hotKey( aArgs.m_defaultHotKey.value_or( 0 ) ),
         m_legacyName( aArgs.m_legacyName.value_or( "" ) ),
-        m_label( aArgs.m_menuText.value_or( wxEmptyString ) ),
-        m_tooltip( aArgs.m_tooltip.value_or( wxEmptyString ) ),
+        m_label( TowxString( aArgs.m_menuText.value_or( "" ) ) ),
+        m_tooltip( TowxString( aArgs.m_tooltip.value_or( "" ) ) ),
         m_icon( aArgs.m_icon.value_or( BITMAPS::INVALID_BITMAP) ),
         m_id( -1 ),
         m_uiid( std::nullopt ),
@@ -86,7 +88,7 @@ TOOL_ACTION::TOOL_ACTION( const TOOL_ACTION_ARGS& aArgs ) :
         m_param = aArgs.m_param;
 
     if( aArgs.m_description.has_value() )
-        m_description = aArgs.m_description;
+        m_description = TowxString( aArgs.m_description.value() );
 
     ACTION_MANAGER::GetActionList().push_back( this );
 }

include/tool/tool_action.h

@@ -29,9 +29,10 @@
 #define __TOOL_ACTION_H
 
 #include <any>
-#include <string>
 #include <cassert>
 #include <optional>
+#include <string>
+#include <string_view>
 
 #include <wx/string.h>
 
@@ -58,6 +59,9 @@ enum TOOL_ACTION_FLAGS
 /**
  * Build up the properties of a TOOL_ACTION in an incremental manner that is static-construction
  * safe.
+ *
+ * Note: This is meant to be constructed and immediately passed into the TOOL_ACTION constructor.
+ *       Construction should not be delayed, since this only retains pointers to the strings used.
  */
 class TOOL_ACTION_ARGS
 {
@@ -69,7 +73,7 @@ public:
      *
      * This is a required property.
      */
-    TOOL_ACTION_ARGS& Name( std::string aName )
+    TOOL_ACTION_ARGS& Name( const std::string_view& aName )
     {
         m_name = aName;
         return *this;
@@ -98,7 +102,7 @@ public:
      *
      * This property is only needed for existing actions and shouldn't be used in new actions.
      */
-    TOOL_ACTION_ARGS& LegacyHotkeyName( std::string aLegacyName )
+    TOOL_ACTION_ARGS& LegacyHotkeyName( const std::string_view& aLegacyName )
     {
         m_legacyName = aLegacyName;
         return *this;
@@ -107,7 +111,7 @@ public:
     /**
      *The string to use when displaying the action in a menu.
      */
-    TOOL_ACTION_ARGS& MenuText( wxString aMenuText )
+    TOOL_ACTION_ARGS& MenuText( const std::string_view& aMenuText )
     {
         m_menuText = aMenuText;
         return *this;
@@ -116,7 +120,7 @@ public:
     /**
      * The string to use as a tooltip for the action in menus and toolbars.
      */
-    TOOL_ACTION_ARGS& Tooltip( wxString aTooltip )
+    TOOL_ACTION_ARGS& Tooltip( const std::string_view& aTooltip )
     {
         m_tooltip = aTooltip;
         return *this;
@@ -125,7 +129,7 @@ public:
     /**
      * The description of the action.
      */
-    TOOL_ACTION_ARGS& Description( wxString aDescription )
+    TOOL_ACTION_ARGS& Description( const std::string_view& aDescription )
     {
         m_description = aDescription;
         return *this;
@@ -172,18 +176,18 @@ protected:
     // Let the TOOL_ACTION constructor have direct access to the members here
     friend class TOOL_ACTION;
 
-    std::optional<std::string>          m_name;
+    std::optional<std::string_view>     m_name;
     std::optional<TOOL_ACTION_SCOPE>    m_scope;
     std::optional<TOOL_ACTION_FLAGS>    m_flags;
 
     std::optional<int>                  m_uiid;
 
     std::optional<int>                  m_defaultHotKey;
-    std::optional<std::string>          m_legacyName;
+    std::optional<std::string_view>     m_legacyName;
 
-    std::optional<wxString>             m_menuText;
-    std::optional<wxString>             m_tooltip;
-    std::optional<wxString>             m_description;
+    std::optional<std::string_view>     m_menuText;
+    std::optional<std::string_view>     m_tooltip;
+    std::optional<std::string_view>     m_description;
 
     std::optional<BITMAPS>              m_icon;
 

libs/core/include/core/wx_stl_compat.h

@@ -20,6 +20,8 @@
 #ifndef KICAD_WX_STL_COMPAT_H
 #define KICAD_WX_STL_COMPAT_H
 
+#include <string_view>
+
 #include <wx/gdicmn.h>
 #include <wx/string.h>
 
@@ -55,6 +57,11 @@ namespace std
     };
 }
 
+/***
+ * Helper function to construct a wxString from a std::string_view.
+ */
+wxString TowxString( const std::string_view& view );
+
 /**
  * Helper function to print the given wxSize to a stream.
  *

libs/core/wx_stl_compat.cpp

@@ -39,6 +39,12 @@ bool std::less<wxPoint>::operator()( const wxPoint& aA, const wxPoint& aB ) cons
 #endif
 
 
+wxString TowxString( const std::string_view& view )
+{
+    return wxString( view.data(), view.length() );
+}
+
+
 std::ostream& operator<<( std::ostream& out, const wxSize& size )
 {
     out << " width=\"" << size.GetWidth() << "\" height=\"" << size.GetHeight() << "\"";