refactor option handling (#1351)

* rough in Option class

* testo passes

* rough in OptionBool

* more OptionBool use

* more OptionBool

* fix isEmpty

* store qstrings

* dont return const

* rename

* convert a few lost ARGTYPE_BOOLs to OptionBool.

* utilize avalable QString for options.

* obsolete argvalptr

* Option is abstract class

* eliminate special handling of ARGTYPE_BOOL without default in vecs.

* fix #982 ...

garmin, ozi, xcsv mkshort options whitespace_ok, must_unique broken since 2006.

* test to make sure OptionBool iff ARGTYPE_BOOL

* use OptionBool for all ARGTYPE_BOOL options.

* correct OptionBool type.

* delete unused parameterized Option ctors.

* format and comment option.h

* nodiscard for class Option.

* vecs nullptr checks

* have Vecs::validate_args check argval isn't nullptr.

* catch up includes

* clarify comment

* fix introduced bug caught by tidy.

Author: tsteven4

arcdist.cc

@@ -24,6 +24,7 @@
 #include <cmath>                  // for round
 #include <cstdio>                 // for printf, sscanf
 #include <cstdlib>                // for strtod
+#include <tuple>                  // for tie, tuple
 
 #include <QByteArray>             // for QByteArray
 #include <QString>                // for QString
@@ -112,7 +113,7 @@ void ArcDistanceFilter::process()
     QString line;
 
     gpsbabel::TextStream stream;
-    stream.open(arcfileopt, QIODevice::ReadOnly, MYNAME);
+    stream.open(arcfileopt.get(), QIODevice::ReadOnly, MYNAME);
 
     auto* arcpt1 = new Waypoint;
     auto* arcpt2 = new Waypoint;
@@ -157,7 +158,7 @@ void ArcDistanceFilter::process()
     if (wp->extra_data) {
       auto* ed = (extra_data*) wp->extra_data;
       wp->extra_data = nullptr;
-      if ((ed->distance >= pos_dist) == (exclopt == nullptr)) {
+      if ((ed->distance >= pos_dist) == !exclopt) {
         wp->wpt_flags.marked_for_deletion = 1;
         removed++;
       } else if (projectopt) {

arcdist.h

@@ -22,10 +22,13 @@
 #ifndef ARCDIST_H_INCLUDED_
 #define ARCDIST_H_INCLUDED_
 
-#include <QVector>         // for QVector
+#include <QList>     // for QList
+#include <QString>   // for QString
+#include <QVector>   // for QVector
 
 #include "defs.h"    // for ARG_NOMINMAX, ARGTYPE_BOOL, Waypoint (ptr only)
 #include "filter.h"  // for Filter
+#include "option.h"  // for OptionBool, OptionCString
 
 #if FILTERS_ENABLED
 
@@ -58,13 +61,13 @@ class ArcDistanceFilter:public Filter
   /* Data Members */
 
   double pos_dist{};
-  char* distopt = nullptr;
-  char* arcfileopt = nullptr;
-  char* rteopt = nullptr;
-  char* trkopt = nullptr;
-  char* exclopt = nullptr;
-  char* ptsopt = nullptr;
-  char* projectopt = nullptr;
+  OptionCString distopt;
+  OptionCString arcfileopt;
+  OptionBool rteopt;
+  OptionBool trkopt;
+  OptionBool exclopt;
+  OptionBool ptsopt;
+  OptionBool projectopt;
 
   QVector<arglist_t> args = {
     {

bend.h

@@ -23,10 +23,13 @@
 #ifndef BEND_H_INCLUDED_
 #define BEND_H_INCLUDED_
 
-#include <QVector>         // for QVector
+#include <QList>     // for QList
+#include <QString>   // for QString
+#include <QVector>   // for QVector
 
 #include "defs.h"    // for route_head (ptr only), ARGTYPE_FLOAT, ARG_NOMINMAX
 #include "filter.h"  // for Filter
+#include "option.h"  // for OptionCString
 
 #if FILTERS_ENABLED
 
@@ -42,8 +45,8 @@ class BendFilter:public Filter
   void deinit() override;
 
 private:
-  char* distopt = nullptr;
-  char* minangleopt = nullptr;
+  OptionCString distopt;
+  OptionCString minangleopt;
 
   double maxDist{};
   double minAngle{};

defs.h

@@ -46,6 +46,7 @@
 #include "geocache.h"                // for Geocache
 #include "formspec.h"                // for FormatSpecificData
 #include "inifile.h"                 // for inifile_t
+#include "option.h"
 #include "session.h"                 // for session_t
 #include "src/core/datetime.h"       // for DateTime
 
@@ -873,13 +874,13 @@ extern posn_status tracking_status;
 
 struct arglist_t {
   const QString argstring;
-  char** const argval{nullptr};
+  Option* const argval{nullptr};
   const QString helpstring;
   const QString defaultvalue;
   const uint32_t argtype{ARGTYPE_UNKNOWN};
   const QString minvalue;    /* minimum value for numeric options */
   const QString maxvalue;    /* maximum value for numeric options */
-  char* argvalptr{nullptr};         /* !!! internal helper. Not used in definitions !!! */
+  char* unused{nullptr};     /* TODO: delete from initialization lists */
 };
 
 enum ff_type {

dg-100.cc

@@ -692,12 +692,12 @@ Dg100Format::rd_deinit()
 void
 Dg100Format::read()
 {
-  if (*erase_only == '1') {
+  if (erase_only) {
     dg100_erase();
     return;
   }
   dg100_getfiles();
-  if (*erase == '1') {
+  if (erase) {
     dg100_erase();
   }
 }

dg-100.h

@@ -30,17 +30,18 @@
 #ifndef DG100_H_INCLUDED_
 #define DG100_H_INCLUDED_
 
-#include <cstdint>           // for uint8_t, int16_t, uint16_t
-#include <cstdio>            // for size_t
+#include <cstdint>    // for uint8_t, int16_t, uint16_t
+#include <cstdio>     // for size_t
 
-#include <QDateTime>         // for QDateTime
-#include <QList>             // for QList
-#include <QString>           // for QString
-#include <QVector>           // for QVector
+#include <QDateTime>  // for QDateTime
+#include <QList>      // for QList
+#include <QString>    // for QString
+#include <QVector>    // for QVector
 
 #include "defs.h"
-#include "format.h"          // for Format
-#include "gbfile.h"          // for gbfile
+#include "format.h"   // for Format
+#include "gbfile.h"   // for gbfile
+#include "option.h"   // for OptionBool
 
 
 class Dg100Format : public Format
@@ -141,8 +142,8 @@ class Dg100Format : public Format
 
   /* GPSBabel integration */
 
-  char* erase{nullptr};
-  char* erase_only{nullptr};
+  OptionBool erase;
+  OptionBool erase_only;
 
   QVector<arglist_t> dg100_args = {
     {

discard.cc

@@ -157,25 +157,25 @@ void DiscardFilter::init()
   }
 
   if (nameopt) {
-    name_regex = generateRegExp(nameopt);
+    name_regex = generateRegExp(nameopt.get());
     if (!name_regex.isValid()) {
       fatal(FatalMsg() << "discard: matchname option is an invalid expression.");
     }
   }
   if (descopt) {
-    desc_regex = generateRegExp(descopt);
+    desc_regex = generateRegExp(descopt.get());
     if (!desc_regex.isValid()) {
       fatal(FatalMsg() << "discard: matchdesc option is an invalid expression.");
     }
   }
   if (cmtopt) {
-    cmt_regex = generateRegExp(cmtopt);
+    cmt_regex = generateRegExp(cmtopt.get());
     if (!cmt_regex.isValid()) {
       fatal(FatalMsg() << "discard: matchcmt option is an invalid expression.");
     }
   }
   if (iconopt) {
-    icon_regex = generateRegExp(iconopt);
+    icon_regex = generateRegExp(iconopt.get());
     if (!icon_regex.isValid()) {
       fatal(FatalMsg() << "discard: matchicon option is an invalid expression.");
     }

discard.h

@@ -22,12 +22,14 @@
 #ifndef DISCARD_H_INCLUDED_
 #define DISCARD_H_INCLUDED_
 
+#include <QList>               // for QList
 #include <QRegularExpression>  // for QRegularExpression
 #include <QString>             // for QString
 #include <QVector>             // for QVector
 
 #include "defs.h"              // for arglist_t, ARG_NOMINMAX, ARGTYPE_BEGIN_REQ, ARGTYPE_STRING, ARGTYPE_BOOL, ARGTYPE_INT, ARGTYPE_FLOAT, route_head, ARGTYPE_END_REQ, Waypoint, gpsdata_type
 #include "filter.h"            // for Filter
+#include "option.h"            // for OptionCString, OptionBool
 
 #if FILTERS_ENABLED
 class DiscardFilter:public Filter
@@ -48,21 +50,21 @@ class DiscardFilter:public Filter
 
   /* Data Members */
 
-  char* hdopopt = nullptr;
-  char* vdopopt = nullptr;
-  char* andopt = nullptr;
-  char* satopt = nullptr;
-  char* fixnoneopt = nullptr;
-  char* fixunknownopt = nullptr;
-  char* eleminopt = nullptr;
-  char* elemaxopt = nullptr;
-  char* nameopt = nullptr;
+  OptionCString hdopopt;
+  OptionCString vdopopt;
+  OptionBool andopt;
+  OptionCString satopt;
+  OptionBool fixnoneopt;
+  OptionBool fixunknownopt;
+  OptionCString eleminopt;
+  OptionCString elemaxopt;
+  OptionCString nameopt;
   QRegularExpression name_regex;
-  char* descopt = nullptr;
+  OptionCString descopt;
   QRegularExpression desc_regex;
-  char* cmtopt = nullptr;
+  OptionCString cmtopt;
   QRegularExpression cmt_regex;
-  char* iconopt = nullptr;
+  OptionCString iconopt;
   QRegularExpression icon_regex;
 
   double hdopf{};

duplicate.h

@@ -22,11 +22,13 @@
 #ifndef DUPLICATE_H_INCLUDED_
 #define DUPLICATE_H_INCLUDED_
 
+#include <QList>     // for QList
 #include <QString>   // for QString
 #include <QVector>   // for QVector
 
 #include "defs.h"    // for ARGTYPE_BOOL, ARG_NOMINMAX, Waypoint (ptr only)
 #include "filter.h"  // for Filter
+#include "option.h"  // for OptionBool
 
 #if FILTERS_ENABLED
 
@@ -41,10 +43,10 @@ class DuplicateFilter:public Filter
   void process() override;
 
 private:
-  char* snopt = nullptr;
-  char* lcopt = nullptr;
-  char* purge_duplicates = nullptr;
-  char* correct_coords = nullptr;
+  OptionBool snopt;
+  OptionBool lcopt;
+  OptionBool purge_duplicates;
+  OptionBool correct_coords;
 
   QVector<arglist_t> args = {
     {

exif.cc

@@ -1178,7 +1178,7 @@ ExifFormat::exif_find_wpt_by_name(const Waypoint* wpt)
 {
   if (exif_wpt_ref != nullptr) {
     return;
-  } else if ((wpt->shortname != nullptr) && (case_ignore_strcmp(wpt->shortname, opt_name) == 0)) {
+  } else if ((wpt->shortname != nullptr) && (case_ignore_strcmp(wpt->shortname, opt_name.get()) == 0)) {
     exif_wpt_ref = wpt;
   }
 }
@@ -1502,7 +1502,7 @@ ExifFormat::wr_deinit()
   gbfclose(fout_);
 
   if (exif_success) {
-    if (*opt_overwrite == '1') {
+    if (opt_overwrite) {
       QFile::remove(exif_fout_name);
       QFile::rename(tmpname, exif_fout_name);
     }
@@ -1530,7 +1530,7 @@ ExifFormat::write()
       track_disp_all(nullptr, nullptr, exif_find_wpt_by_name_lambda);
     }
     if (exif_wpt_ref == nullptr) {
-      warning(MYNAME ": No matching point with name \"%s\" found.\n", opt_name);
+      warning(MYNAME ": No matching point with name \"%s\" found.\n", qPrintable(opt_name.get()));
     }
   } else {
     auto exif_find_wpt_by_time_lambda = [this](const Waypoint* waypointp)->void {

exif.h

@@ -36,19 +36,21 @@
 #ifndef EXIF_H_INCLUDED_
 #define EXIF_H_INCLUDED_
 
-#include <QDate>      // for QDate
-#include <QDateTime>  // for QDateTime
-#include <QList>      // for QList
-#include <QString>    // for QString
-#include <QTime>      // for QTime
-#include <QVariant>   // for QVariant
-#include <QVector>    // for QVector
+#include <QByteArray>  // for QByteArray
+#include <QDate>       // for QDate
+#include <QDateTime>   // for QDateTime
+#include <QList>       // for QList
+#include <QString>     // for QString
+#include <QTime>       // for QTime
+#include <QVariant>    // for QVariant
+#include <QVector>     // for QVector
 
-#include <cstdint>    // for uint32_t, uint16_t, uint8_t, int16_t, int32_t
+#include <cstdint>     // for uint32_t, uint16_t, uint8_t, int16_t, int32_t
 
-#include "defs.h"     // for arglist_t, ff_cap, Waypoint, ARG_NOMINMAX, ARGTYPE_BOOL, ff_cap_none, ARGTYPE_INT, ARGTYPE_STRING, ff_cap_read, ff_cap_write, ff_type, ff_type_file
-#include "format.h"   // for Format
-#include "gbfile.h"   // for gbfile, gbsize_t
+#include "defs.h"      // for arglist_t, ff_cap, Waypoint, ARG_NOMINMAX, ARGTYPE_BOOL, ff_cap_none, ARGTYPE_INT, ARGTYPE_STRING, ff_cap_read, ff_cap_write, ff_type, ff_type_file
+#include "format.h"    // for Format
+#include "gbfile.h"    // for gbfile, gbsize_t
+#include "option.h"    // for OptionCString, OptionBool
 
 
 class ExifFormat : public Format
@@ -201,11 +203,11 @@ class ExifFormat : public Format
   char exif_success{};
   QString exif_fout_name;
 
-  char* opt_filename{};
-  char* opt_overwrite{};
-  char* opt_frame{};
-  char* opt_name{};
-  char* opt_offsettime{};
+  OptionBool opt_filename;
+  OptionBool opt_overwrite;
+  OptionCString opt_frame;
+  OptionCString opt_name;
+  OptionCString opt_offsettime;
 
   QVector<arglist_t> exif_args = {
     { "filename", &opt_filename, "Set waypoint name to source filename", "Y", ARGTYPE_BOOL, ARG_NOMINMAX, nullptr },

filter_vecs.cc

@@ -31,6 +31,7 @@
 #include <algorithm>        // for sort
 #include <cassert>          // for assert
 #include <cstdio>           // for printf
+#include <type_traits>      // for is_base_of
 
 #include "arcdist.h"        // for ArcDistanceFilter
 #include "bend.h"           // for BendFilter
@@ -216,9 +217,9 @@ void FilterVecs::prepare_filter(const fltinfo_t& fltdata)
         qtemp = inifile_readstr(global_opts.inifile, "Common filter settings", arg.argstring);
       }
       if (qtemp.isNull()) {
-        Vecs::assign_option(fltdata.fltname, &arg, arg.defaultvalue);
+        Vecs::assign_option(fltdata.fltname, arg, arg.defaultvalue);
       } else {
-        Vecs::assign_option(fltdata.fltname, &arg, qtemp);
+        Vecs::assign_option(fltdata.fltname, arg, qtemp);
       }
     }
   }
@@ -230,7 +231,7 @@ void FilterVecs::prepare_filter(const fltinfo_t& fltdata)
       for (auto& arg : *args) {
         const QString opt = Vecs::get_option(fltdata.options, arg.argstring);
         if (!opt.isNull()) {
-          Vecs::assign_option(fltdata.fltname, &arg, opt);
+          Vecs::assign_option(fltdata.fltname, arg, opt);
         }
       }
     }
@@ -274,9 +275,6 @@ void FilterVecs::init_filter_vec(Filter* flt)
   QVector<arglist_t>* args = flt->get_args();
   if (args && !args->isEmpty()) {
     assert(args->isDetached());
-    for (auto& arg : *args) {
-      arg.argvalptr = nullptr;
-    }
   }
 }