gnucash master: Rework command line option parsing to store values directly in class member variables

Geert Janssens gjanssens at code.gnucash.org
Fri Jun 5 12:16:32 EDT 2020


Updated	 via  https://github.com/Gnucash/gnucash/commit/422dca54 (commit)
	from  https://github.com/Gnucash/gnucash/commit/d14e2cce (commit)



commit 422dca54e1580744eeb29b9583844ffa9414a1f7
Author: Geert Janssens <geert at kobaltwit.be>
Date:   Fri Jun 5 18:16:25 2020 +0200

    Rework command line option parsing to store values directly in class member variables
    
    The program options libary has a convenience binding for boost::optional
    to indicate whether an option is set or not.
    Use this to store options passed on the command line directly
    in variables for later use. This avoids the need to refer to options
    in several locations using a fixed string (like 'help', 'help-gtk', 'nofile',...)
    
    In addition drop a number of obsolete class member variables.
    They were leftovers from the conversion to c++ and no longer used.

diff --git a/gnucash/gnucash-cli.cpp b/gnucash/gnucash-cli.cpp
index 48e68fc5d..31d68ffc0 100644
--- a/gnucash/gnucash-cli.cpp
+++ b/gnucash/gnucash-cli.cpp
@@ -37,6 +37,7 @@ extern "C" {
 }
 
 #include <boost/locale.hpp>
+#include <boost/optional.hpp>
 #include <iostream>
 
 namespace bl = boost::locale;
@@ -56,9 +57,11 @@ namespace Gnucash {
         void configure_program_options (void);
 
         bool m_add_quotes;
-        std::string m_run_report;
-        std::string m_export_type;
-        std::string m_output_file;
+        boost::optional <std::string> m_namespace;
+
+        boost::optional <std::string> m_run_report;
+        boost::optional <std::string> m_export_type;
+        boost::optional <std::string> m_output_file;
     };
 
 }
@@ -73,23 +76,11 @@ Gnucash::GnucashCli::parse_command_line (int argc, char **argv)
 {
     Gnucash::CoreApp::parse_command_line (argc, argv);
 
-    if (m_log_to_filename.empty())
-        m_log_to_filename.assign("stderr");
-
-    m_add_quotes = m_opt_map["add-price-quotes"].as<bool>();
-
-    if (m_opt_map.count ("namespace"))
-        gnc_prefs_set_namespace_regexp(m_opt_map["namespace"].
-        as<std::string>().c_str());
-
-    if (m_opt_map.count ("run-report"))
-        m_run_report = m_opt_map["run-report"].as<std::string>();
-
-    if (m_opt_map.count ("export-type"))
-        m_export_type = m_opt_map["export-type"].as<std::string>();
+    if (!m_log_to_filename || m_log_to_filename->empty())
+        m_log_to_filename = "stderr";
 
-    if (m_opt_map.count ("output-file"))
-        m_output_file = m_opt_map["output-file"].as<std::string>();
+    if (m_namespace)
+        gnc_prefs_set_namespace_regexp (m_namespace->c_str());
 }
 
 // Define command line options specific to gnucash-cli.
@@ -98,19 +89,19 @@ Gnucash::GnucashCli::configure_program_options (void)
 {
     bpo::options_description quotes_options(_("Price Quotes Retrieval Options"));
     quotes_options.add_options()
-    ("add-price-quotes", bpo::bool_switch(),
+    ("add-price-quotes", bpo::bool_switch (&m_add_quotes),
      _("Add price quotes to given GnuCash datafile."))
-    ("namespace", bpo::value<std::string>(),
+    ("namespace", bpo::value (&m_namespace),
      _("Regular expression determining which namespace commodities will be retrieved"));
     m_opt_desc->add (quotes_options);
 
     bpo::options_description report_options(_("Report Generation Options"));
     report_options.add_options()
-    ("run-report", bpo::value<std::string>(),
+    ("run-report", bpo::value (&m_run_report),
      _("Runs a report\n"))
-    ("export-type", bpo::value<std::string>(),
+    ("export-type", bpo::value (&m_export_type),
      _("Specify export type\n"))
-    ("output-file", bpo::value<std::string>(),
+    ("output-file", bpo::value (&m_output_file),
      _("Output file for report\n"));
     m_opt_desc->add (report_options);
 
@@ -123,26 +114,26 @@ Gnucash::GnucashCli::start ([[maybe_unused]] int argc, [[maybe_unused]] char **a
 
     if (m_add_quotes)
     {
-        if (m_file_to_load.empty())
+        if (!m_file_to_load || m_file_to_load->empty())
         {
             std::cerr << bl::translate("Missing data file parameter") << "\n\n"
                       << *m_opt_desc.get();
             return 1;
         }
         else
-            return Gnucash::add_quotes (m_file_to_load);
+            return Gnucash::add_quotes (*m_file_to_load);
     }
 
-    if (!m_run_report.empty())
+    if (m_run_report && !m_run_report->empty())
     {
-        if (m_file_to_load.empty())
+        if (!m_file_to_load || m_file_to_load->empty())
         {
             std::cerr << bl::translate("Missing data file parameter") << "\n\n"
                       << *m_opt_desc.get();
             return 1;
         }
         else
-            return Gnucash::run_report(m_file_to_load, m_run_report,
+            return Gnucash::run_report(*m_file_to_load, m_run_report,
                                        m_export_type, m_output_file);
     }
     return 1;
diff --git a/gnucash/gnucash-commands.cpp b/gnucash/gnucash-commands.cpp
index bb8ca01a4..e84d25cbf 100644
--- a/gnucash/gnucash-commands.cpp
+++ b/gnucash/gnucash-commands.cpp
@@ -198,23 +198,25 @@ scm_run_report (void *data,
 }
 
 int
-Gnucash::add_quotes (const std::string& uri)
+Gnucash::add_quotes (const bo_str& uri)
 {
-    if (!uri.empty())
-        scm_boot_guile (0, nullptr, scm_add_quotes, (void *)&uri);
+    if (uri && !uri->empty())
+        scm_boot_guile (0, nullptr, scm_add_quotes, (void *)&(*uri));
 
     return 0;
 }
 
 int
-Gnucash::run_report (const std::string& file_to_load,
-                     const std::string& run_report,
-                     const std::string& export_type,
-                     const std::string& output_file)
+Gnucash::run_report (const bo_str& file_to_load,
+                     const bo_str& run_report,
+                     const bo_str& export_type,
+                     const bo_str& output_file)
 {
-    auto args = run_report_args { file_to_load, run_report,
-                                  export_type, output_file };
-    if (!run_report.empty())
+    auto args = run_report_args { file_to_load ? *file_to_load : std::string(),
+                                  run_report ? *run_report : std::string(),
+                                  export_type ? *export_type : std::string(),
+                                  output_file ? *output_file : std::string() };
+    if (run_report && !run_report->empty())
         scm_boot_guile (0, nullptr, scm_run_report, &args);
 
     return 0;
diff --git a/gnucash/gnucash-commands.hpp b/gnucash/gnucash-commands.hpp
index e400aca78..328d18513 100644
--- a/gnucash/gnucash-commands.hpp
+++ b/gnucash/gnucash-commands.hpp
@@ -26,14 +26,17 @@
 #define GNUCASH_COMMANDS_HPP
 
 #include <string>
+#include <boost/optional.hpp>
+
+using bo_str = boost::optional <std::string>;
 
 namespace Gnucash {
 
-    int add_quotes (const std::string& uri);
-    int run_report (const std::string& file_to_load,
-                    const std::string& run_report,
-                    const std::string& export_type,
-                    const std::string& output_file);
+    int add_quotes (const bo_str& uri);
+    int run_report (const bo_str& file_to_load,
+                    const bo_str& run_report,
+                    const bo_str& export_type,
+                    const bo_str& output_file);
 
 }
 #endif
diff --git a/gnucash/gnucash-core-app.cpp b/gnucash/gnucash-core-app.cpp
index 7fba6ddd8..a7bd13457 100644
--- a/gnucash/gnucash-core-app.cpp
+++ b/gnucash/gnucash-core-app.cpp
@@ -350,14 +350,15 @@ load_user_config(void)
 
 
 static void
-gnc_log_init (std::vector <std::string> &log_flags, std::string &log_to_filename)
+gnc_log_init (const boost::optional <std::vector <std::string>> &log_flags,
+              const boost::optional <std::string> &log_to_filename)
 {
-    if (!log_to_filename.empty())
+    if (log_to_filename && !log_to_filename->empty())
     {
 #ifdef __MINGW64__
-        auto *utf8_filename = g_utf16_to_utf8 (log_to_filename, -1, NULL, NULL, NULL);
+        auto *utf8_filename = g_utf16_to_utf8 (log_to_filename->c_str(), -1, NULL, NULL, NULL);
 #else
-        auto utf8_filename = log_to_filename.c_str();
+        auto utf8_filename = log_to_filename->c_str();
 #endif
 
         qof_log_init_filename_special (utf8_filename);
@@ -392,9 +393,9 @@ gnc_log_init (std::vector <std::string> &log_flags, std::string &log_to_filename
         qof_log_parse_log_config (log_config_filename);
     g_free (log_config_filename);
 
-    if (!log_flags.empty())
+    if (log_flags && !log_flags->empty())
     {
-        for (auto log_flag : log_flags)
+        for (auto log_flag : *log_flags)
         {
             if (log_flag.empty () ||
                 log_flag[0] == '=' ||
@@ -516,9 +517,9 @@ Gnucash::CoreApp::CoreApp (const char* app_name)
     m_app_name = std::string(app_name);
 
     // Now that gettext is properly initialized, set our help tagline.
-    tagline = bl::translate("- GnuCash, accounting for personal and small business finance").str(gnc_get_locale());
+    m_tagline = bl::translate("- GnuCash, accounting for personal and small business finance").str(gnc_get_locale());
     m_opt_desc = std::make_unique<bpo::options_description>
-        ((bl::format (bl::gettext ("{1} [options] [datafile]")) % m_app_name).str() + std::string(" ") + tagline);
+        ((bl::format (bl::gettext ("{1} [options] [datafile]")) % m_app_name).str() + std::string(" ") + m_tagline);
     add_common_program_options();
 }
 
@@ -544,7 +545,7 @@ Gnucash::CoreApp::parse_command_line (int argc, char **argv)
         exit(1);
     }
 
-    if (m_opt_map["version"].as<bool>())
+    if (m_show_version)
     {
         bl::format rel_fmt (bl::translate ("GnuCash {1}"));
         bl::format dev_fmt (bl::translate ("GnuCash {1} development version"));
@@ -558,32 +559,17 @@ Gnucash::CoreApp::parse_command_line (int argc, char **argv)
         exit(0);
     }
 
-    if (m_opt_map["help"].as<bool>())
+    if (m_show_help)
     {
         std::cout << *m_opt_desc.get() << "\n";
         exit(0);
     }
 
-    gnc_prefs_set_debugging (m_opt_map["debug"].as<bool>());
-    gnc_prefs_set_extra (m_opt_map["extra"].as<bool>());
+    gnc_prefs_set_debugging (m_debug);
+    gnc_prefs_set_extra (m_extra);
 
-    if (m_opt_map.count ("gsettings-prefix"))
-        gnc_gsettings_set_prefix (m_opt_map["gsettings-prefix"].
-            as<std::string>().c_str());
-
-    if (m_opt_map.count ("log"))
-        m_log_flags = m_opt_map["log"].
-            as<std::vector <std::string>>();
-
-    if (m_opt_map.count ("logto"))
-        m_log_to_filename = m_opt_map["logto"].
-            as<std::string>().c_str();
-
-    if (m_opt_map.count ("input-file"))
-        m_file_to_load = m_opt_map["input-file"].as<std::string>();
-
-    if (args_remaining)
-        file_to_load = args_remaining[0];
+    if (m_gsettings_prefix)
+        gnc_gsettings_set_prefix (m_gsettings_prefix->c_str());
 }
 
 /* Define command line options common to all gnucash binaries. */
@@ -592,21 +578,21 @@ Gnucash::CoreApp::add_common_program_options (void)
 {
     bpo::options_description common_options(_("Common Options"));
     common_options.add_options()
-        ("help,h", bpo::bool_switch(),
+        ("help,h", bpo::bool_switch (&m_show_help),
          _("Show this help message"))
-        ("version,v", bpo::bool_switch(),
+        ("version,v", bpo::bool_switch (&m_show_version),
          _("Show GnuCash version"))
-        ("debug", bpo::bool_switch(),
+        ("debug", bpo::bool_switch (&m_debug),
          _("Enable debugging mode: provide deep detail in the logs.\nThis is equivalent to: --log \"=info\" --log \"qof=info\" --log \"gnc=info\""))
-        ("extra", bpo::bool_switch(),
+        ("extra", bpo::bool_switch(&m_extra),
          _("Enable extra/development/debugging features."))
-        ("log", bpo::value< std::vector<std::string> >(),
+        ("log", bpo::value (&m_log_flags),
          _("Log level overrides, of the form \"modulename={debug,info,warn,crit,error}\"\nExamples: \"--log qof=debug\" or \"--log gnc.backend.file.sx=info\"\nThis can be invoked multiple times."))
-        ("logto", bpo::value<std::string>(),
+        ("logto", bpo::value (&m_log_to_filename),
          _("File to log into; defaults to \"/tmp/gnucash.trace\"; can be \"stderr\" or \"stdout\"."))
-        ("gsettings-prefix", bpo::value<std::string>(),
+        ("gsettings-prefix", bpo::value (&m_gsettings_prefix),
          _("Set the prefix for gsettings schemas for gsettings queries. This can be useful to have a different settings tree while debugging."))
-        ("input-file", bpo::value<std::string>(),
+        ("input-file", bpo::value (&m_file_to_load),
          _("[datafile]"));
 
         m_pos_opt_desc.add("input-file", -1);
diff --git a/gnucash/gnucash-core-app.hpp b/gnucash/gnucash-core-app.hpp
index 4e8926836..8b99252f5 100644
--- a/gnucash/gnucash-core-app.hpp
+++ b/gnucash/gnucash-core-app.hpp
@@ -27,6 +27,7 @@
 #ifdef __MINGW32__
 #undef _GLIBCXX_USE_C99_MATH_TR1 // Avoid cmath missing function decl.
 #endif
+#include <boost/optional.hpp>
 #include <boost/program_options.hpp>
 #include <string>
 #include <vector>
@@ -45,11 +46,10 @@ public:
     void start (void);
 
 protected:
-    int gtk_show_help = 0;
     std::string m_app_name;
-    std::string tagline;
-    std::string m_file_to_load;
-    std::string m_log_to_filename;
+    std::string m_tagline;
+    boost::optional <std::string> m_log_to_filename;
+    boost::optional <std::string> m_file_to_load;
 
     std::unique_ptr<bpo::options_description> m_opt_desc;
     bpo::variables_map m_opt_map;
@@ -59,17 +59,13 @@ private:
     void add_common_program_options (void);
 
     /* Command-line option variables */
-    std::vector <std::string> m_log_flags;
+    bool m_show_help = false;
+    bool m_show_version = false;
+    bool m_debug = false;
+    bool m_extra = false;
+    boost::optional <std::string> m_gsettings_prefix;
+    boost::optional <std::vector <std::string>> m_log_flags;
 
-    int gnucash_show_version = 0;
-    int debugging = 0;
-    int extra = 0;
-    int nofile = 0;
-    const char *gsettings_prefix = nullptr;
-    const char *add_quotes_file = nullptr;
-    char *namespace_regexp = nullptr;
-    const char *file_to_load = nullptr;
-    char **args_remaining = nullptr;
     char *sys_locale = nullptr;
 };
 
diff --git a/gnucash/gnucash.cpp b/gnucash/gnucash.cpp
index 869dbc0d5..182e1fcf7 100644
--- a/gnucash/gnucash.cpp
+++ b/gnucash/gnucash.cpp
@@ -60,6 +60,7 @@ extern "C" {
 }
 
 #include <boost/locale.hpp>
+#include <boost/optional.hpp>
 #include <iostream>
 #include <gnc-locale-utils.hpp>
 
@@ -279,8 +280,12 @@ namespace Gnucash {
     private:
         void configure_program_options (void);
 
-        std::string m_gtk_help_msg;
+        bool m_nofile = false;
+        bool m_show_help_gtk = false;
         bool m_add_quotes; // Deprecated will be removed in gnucash 5.0
+        boost::optional <std::string> m_namespace; // Deprecated will be removed in gnucash 5.0
+
+        std::string m_gtk_help_msg;
     };
 
 }
@@ -296,17 +301,14 @@ Gnucash::Gnucash::parse_command_line (int argc, char **argv)
 {
     Gnucash::CoreApp::parse_command_line (argc, argv);
 
-    if (m_opt_map["help-gtk"].as<bool>())
+    if (m_show_help_gtk)
     {
         std::cout << m_gtk_help_msg;
         exit(0);
     }
 
-    m_add_quotes = m_opt_map["add-price-quotes"].as<bool>();
-
-    if (m_opt_map.count ("namespace"))
-        gnc_prefs_set_namespace_regexp (m_opt_map["namespace"].
-            as<std::string>().c_str());
+    if (m_namespace)
+        gnc_prefs_set_namespace_regexp (m_namespace->c_str());
 }
 
 // Define command line options specific to gnucash.
@@ -317,7 +319,7 @@ Gnucash::Gnucash::configure_program_options (void)
     // for gtk's options. The options themselves are already parsed out by
     // gtk_init_check by the time this function is called though. So it really only
     // serves to be able to display a help message.
-    auto context = g_option_context_new (tagline.c_str());
+    auto context = g_option_context_new (m_tagline.c_str());
     auto gtk_options = gtk_get_option_group(FALSE);
     g_option_context_add_group (context, gtk_options);
     m_gtk_help_msg = g_option_context_get_help (context, FALSE, gtk_options);
@@ -325,18 +327,18 @@ Gnucash::Gnucash::configure_program_options (void)
 
     bpo::options_description app_options(_("Application Options"));
     app_options.add_options()
-    ("nofile", bpo::bool_switch(),
+    ("nofile", bpo::bool_switch (&m_nofile),
      N_("Do not load the last file opened"))
-    ("help-gtk",  bpo::bool_switch(),
+    ("help-gtk",  bpo::bool_switch (&m_show_help_gtk),
      _("Show help for gtk options"));
 
     bpo::options_description depr_options(_("Deprecated Options"));
     depr_options.add_options()
-    ("add-price-quotes", bpo::bool_switch(),
+    ("add-price-quotes", bpo::bool_switch (&m_add_quotes),
      _("Add price quotes to given GnuCash datafile.\n"
         "Note this option has been deprecated and will be removed in GnuCash 5.0.\n"
         "Please use \"gnucash-cli --add-price-quotes\" instead."))
-    ("namespace", bpo::value<std::string>(),
+    ("namespace", bpo::value (&m_namespace),
      _("Regular expression determining which namespace commodities will be retrieved.\n"
        "Note this option has been deprecated and will be removed in GnuCash 5.0.\n"
         "Please use \"gnucash-cli --add-price-quotes\" instead."));
@@ -355,7 +357,7 @@ Gnucash::Gnucash::start ([[maybe_unused]] int argc, [[maybe_unused]] char **argv
     {
         std::cerr << bl::translate ("The '--add-price-quotes' option to gnucash has been deprecated and will be removed in GnuCash 5.0. "
                                     "Please use 'gnucash-cli --add-price-quotes' instead.") << "\n";
-        if (m_file_to_load.empty())
+        if (!m_file_to_load || m_file_to_load->empty())
         {
             std::cerr << bl::translate("Missing data file parameter") << "\n\n"
             << *m_opt_desc.get();
@@ -372,8 +374,8 @@ Gnucash::Gnucash::start ([[maybe_unused]] int argc, [[maybe_unused]] char **argv
     gnc_gui_init();
 
     auto user_file_spec = t_file_spec {
-        m_opt_map["nofile"].as<bool>(),
-        m_file_to_load.c_str()};
+        m_nofile,
+        m_file_to_load ? m_file_to_load->c_str() : ""};
     scm_boot_guile (argc, argv, scm_run_gnucash, &user_file_spec);
 
     return 0;



Summary of changes:
 gnucash/gnucash-cli.cpp      | 49 +++++++++++++++---------------------
 gnucash/gnucash-commands.cpp | 22 ++++++++--------
 gnucash/gnucash-commands.hpp | 13 ++++++----
 gnucash/gnucash-core-app.cpp | 60 +++++++++++++++++---------------------------
 gnucash/gnucash-core-app.hpp | 24 ++++++++----------
 gnucash/gnucash.cpp          | 32 ++++++++++++-----------
 6 files changed, 90 insertions(+), 110 deletions(-)



More information about the gnucash-changes mailing list