extcap: Group arguments

Group arguments together to better present them, as well as to
have the possibility to better facilitate settings categories.

The order of tabs is defined by the numbering of arguments and
their appearance. If no tab can be found or no group has been
defined for the argument, a default tab will be added.

Change-Id: I032881193e09d4ad5d65c9f73fede87695acdace
Reviewed-on: https://code.wireshark.org/review/27054
Petri-Dish: Roland Knall <rknall@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>

6 files changed, 177 insertions, 34 deletions

diff --git a/doc/extcap_example.py b/doc/extcap_example.py
index 3e6b96b84c..2080e48af9 100755
--- a/doc/extcap_example.py
+++ b/doc/extcap_example.py
@@ -117,14 +117,14 @@ def extcap_config(interface, option):
     args.append ( (2, '--verify', 'Verify', 'Verify package content', 'boolflag', '{default=yes}') )
     args.append ( (3, '--remote', 'Remote Channel', 'Remote Channel Selector', 'selector', '{reload=true}{placeholder=Load interfaces ...}'))
     args.append ( (4, '--fake_ip', 'Fake IP Address', 'Use this ip address as sender', 'string', '{save=false}{validation=\\b(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\b}'))
-    args.append ( (5, '--ltest', 'Long Test', 'Long Test Value', 'long', '{default=123123123123123123}'))
-    args.append ( (6, '--d1test', 'Double 1 Test', 'Long Test Value', 'double', '{default=123.456}'))
-    args.append ( (7, '--d2test', 'Double 2 Test', 'Long Test Value', 'double', '{default= 123,456}'))
+    args.append ( (5, '--ltest', 'Long Test', 'Long Test Value', 'long', '{default=123123123123123123}{group=Numeric Values}'))
+    args.append ( (6, '--d1test', 'Double 1 Test', 'Long Test Value', 'double', '{default=123.456}{group=Numeric Values}'))
+    args.append ( (7, '--d2test', 'Double 2 Test', 'Long Test Value', 'double', '{default= 123,456}{group=Numeric Values}'))
     args.append ( (8, '--password', 'Password', 'Package message password', 'password', '') )
-    args.append ( (9, '--ts', 'Start Time', 'Capture start time', 'timestamp', '') )
-    args.append ( (10, '--logfile', 'Log File Test', 'The Log File Test', 'fileselect', '') )
-    args.append ( (11, '--radio', 'Radio Test', 'Radio Test Value', 'radio', '') )
-    args.append ( (12, '--multi', 'MultiCheck Test', 'MultiCheck Test Value', 'multicheck', '') )
+    args.append ( (9, '--ts', 'Start Time', 'Capture start time', 'timestamp', '{group=Time / Log}') )
+    args.append ( (10, '--logfile', 'Log File Test', 'The Log File Test', 'fileselect', '{group=Time / Log}') )
+    args.append ( (11, '--radio', 'Radio Test', 'Radio Test Value', 'radio', '{group=Selection}') )
+    args.append ( (12, '--multi', 'MultiCheck Test', 'MultiCheck Test Value', 'multicheck', '{group=Selection}') )
 
     if ( option == "remote" ):
         values.append ( (3, "if1", "Remote Interface 1", "false" ) )
diff --git a/extcap_parser.c b/extcap_parser.c
index 75d248abf1..411e2522be 100644
--- a/extcap_parser.c
+++ b/extcap_parser.c
@@ -169,6 +169,8 @@ static extcap_token_sentence *extcap_tokenize_sentence(const gchar *s) {
                 param_type = EXTCAP_PARAM_FILE_MUSTEXIST;
             } else if (g_ascii_strcasecmp(arg, "fileext") == 0) {
                 param_type = EXTCAP_PARAM_FILE_EXTENSION;
+            } else if (g_ascii_strcasecmp(arg, "group") == 0) {
+                param_type = EXTCAP_PARAM_GROUP;
             } else if (g_ascii_strcasecmp(arg, "name") == 0) {
                 param_type = EXTCAP_PARAM_NAME;
             } else if (g_ascii_strcasecmp(arg, "enabled") == 0) {
@@ -251,6 +253,7 @@ void extcap_free_arg(extcap_arg *a) {
     g_free(a->placeholder);
     g_free(a->fileextension);
     g_free(a->regexp);
+    g_free(a->group);
     g_free(a->device_name);
 
     if (a->range_start != NULL)
@@ -462,6 +465,11 @@ static extcap_arg *extcap_parse_arg_sentence(GList *args, extcap_token_sentence
             target_arg->regexp = g_strdup(param_value);
         }
 
+        if ((param_value = (gchar *)g_hash_table_lookup(s->param_list, ENUM_KEY(EXTCAP_PARAM_GROUP)))
+                != NULL) {
+            target_arg->group = g_strdup(param_value);
+        }
+
         if ((param_value = (gchar *)g_hash_table_lookup(s->param_list, ENUM_KEY(EXTCAP_PARAM_REQUIRED)))
                 != NULL) {
             target_arg->is_required = g_regex_match_simple(EXTCAP_BOOLEAN_REGEX, param_value, G_REGEX_CASELESS, (GRegexMatchFlags)0);
diff --git a/extcap_parser.h b/extcap_parser.h
index 9800b897cf..a05f4de6ae 100644
--- a/extcap_parser.h
+++ b/extcap_parser.h
@@ -62,6 +62,7 @@ typedef enum {
     EXTCAP_PARAM_ENABLED,
     EXTCAP_PARAM_FILE_MUSTEXIST,
     EXTCAP_PARAM_FILE_EXTENSION,
+    EXTCAP_PARAM_GROUP,
     EXTCAP_PARAM_PARENT,
     EXTCAP_PARAM_REQUIRED,
     EXTCAP_PARAM_RELOAD,
@@ -113,6 +114,8 @@ typedef struct _extcap_arg {
 
     gchar * regexp;
 
+    gchar * group;
+
     extcap_arg_type arg_type;
 
     extcap_complex *range_start;
diff --git a/ui/qt/extcap_argument.cpp b/ui/qt/extcap_argument.cpp
index 1c46f0d857..e06cbbbc90 100644
--- a/ui/qt/extcap_argument.cpp
+++ b/ui/qt/extcap_argument.cpp
@@ -594,7 +594,7 @@ void ExtcapValue::setChildren(ExtcapValueList children)
 }
 
 ExtcapArgument::ExtcapArgument(QObject *parent) :
-        QObject(parent), _argument(0), _label(0),
+        QObject(parent), _argument(0), _label(0), _number(0),
         label_style(QString("QLabel { color: %1; }"))
 {
 }
@@ -603,6 +603,8 @@ ExtcapArgument::ExtcapArgument(extcap_arg * argument, QObject *parent) :
         QObject(parent), _argument(argument), _label(0),
         label_style(QString("QLabel { color: %1; }"))
 {
+    _number = argument->arg_num;
+
     if ( _argument->values != 0 )
     {
         ExtcapValueList elements = loadValues(QString(""));
@@ -615,6 +617,8 @@ ExtcapArgument::ExtcapArgument(const ExtcapArgument &obj) :
         QObject(obj.parent()), _argument(obj._argument), _label(0),
         label_style(QString("QLabel { color: %1; }"))
 {
+    _number = obj._argument->arg_num;
+
     if ( _argument->values != 0 )
     {
         ExtcapValueList elements = loadValues(QString(""));
@@ -754,6 +758,19 @@ QString ExtcapArgument::defaultValue()
     return QString();
 }
 
+QString ExtcapArgument::group() const
+{
+    if ( _argument != 0 && _argument->group != 0 )
+        return QString(_argument->group);
+
+    return QString();
+}
+
+int ExtcapArgument::argNr() const
+{
+    return _number;
+}
+
 QString ExtcapArgument::prefKey(const QString & device_name)
 {
     struct preference * pref = NULL;
diff --git a/ui/qt/extcap_argument.h b/ui/qt/extcap_argument.h
index a24f25fd0f..07813f9f2a 100644
--- a/ui/qt/extcap_argument.h
+++ b/ui/qt/extcap_argument.h
@@ -94,6 +94,9 @@ public:
 
     void resetValue();
 
+    virtual QString group() const;
+    virtual int argNr() const;
+
     static ExtcapArgument * create(extcap_arg * argument = Q_NULLPTR, QObject * parent = Q_NULLPTR);
 
 Q_SIGNALS:
@@ -110,6 +113,7 @@ protected:
 
     extcap_arg * _argument;
     QLabel * _label;
+    int _number;
 
     const QString label_style;
 
diff --git a/ui/qt/extcap_options_dialog.cpp b/ui/qt/extcap_options_dialog.cpp
index 14ae53f1e5..3c69f99ea6 100644
--- a/ui/qt/extcap_options_dialog.cpp
+++ b/ui/qt/extcap_options_dialog.cpp
@@ -17,12 +17,13 @@
 #include <wireshark_application.h>
 
 #include <QMessageBox>
-#include <QMap>
+#include <QHash>
 #include <QHBoxLayout>
 #include <QVBoxLayout>
 #include <QGridLayout>
 #include <QUrl>
 #include <QDesktopServices>
+#include <QTabWidget>
 
 #include "ringbuffer.h"
 #include "ui/capture_ui_utils.h"
@@ -244,20 +245,79 @@ void ExtcapOptionsDialog::updateWidgets()
     /* find existing layout */
     if (ui->verticalLayout->children().count() > 0)
     {
-        QGridLayout * layout = (QGridLayout *)ui->verticalLayout->itemAt(0);
-        ui->verticalLayout->removeItem(layout);
         ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(false);
+        QWidget * item = ui->verticalLayout->itemAt(0)->widget();
+        if ( item )
+        {
+            ui->verticalLayout->removeItem(ui->verticalLayout->itemAt(0));
+            delete item;
+        }
     }
 
-    QGridLayout * layout = new QGridLayout();
+    QHash<QString, QWidget *> layouts;
 
     /* Load all extcap arguments */
     loadArguments();
 
+    /* exit if no arguments have been found. This is a precaution, it should
+     * never happen, that this dialog get's called without any arguments */
+    if ( extcapArguments.count() == 0 )
+    {
+        ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true);
+        return;
+    }
+
+    QStringList groupKeys;
+    QString defaultKeyName(tr("Default"));
+    /* QMap sorts keys, therefore the groups are sorted by appearance */
+    QMap<int, QString> groups;
+
+    /* Look for all necessary tabs */
     ExtcapArgumentList::iterator iter = extcapArguments.begin();
     while ( iter != extcapArguments.end() )
     {
         argument = (ExtcapArgument *)(*iter);
+        QString groupKey = argument->group();
+        if ( groupKey.length() > 0 )
+        {
+            if ( ! groups.values().contains(groupKey) )
+                groups.insert(argument->argNr(), groupKey);
+        }
+        else if ( ! groups.keys().contains(0) )
+        {
+            groups.insert(0, defaultKeyName);
+            groupKey = defaultKeyName;
+        }
+
+        if ( ! layouts.keys().contains(groupKey) )
+        {
+            QWidget * tabWidget = new QWidget(this);
+            QGridLayout * tabLayout = new QGridLayout(tabWidget);
+            tabWidget->setLayout(tabLayout);
+
+            layouts.insert(groupKey, tabWidget);
+        }
+
+        ++iter;
+    }
+    groupKeys << groups.values();
+
+    /* Iterate over all arguments and do the following:
+     *  1. create the label for each element
+     *  2. create an editor for each element
+     *  3. add both to the layout for the tab widget
+     */
+    iter = extcapArguments.begin();
+    while ( iter != extcapArguments.end() )
+    {
+        argument = (ExtcapArgument *)(*iter);
+        QString groupKey = defaultKeyName;
+        if ( argument->group().length() > 0 )
+            groupKey = argument->group();
+
+        Q_ASSERT(layouts.keys().contains(groupKey));
+
+        QGridLayout * layout = ((QGridLayout *)layouts[groupKey]->layout());
         lblWidget = argument->createLabel((QWidget *)this);
         if ( lblWidget != NULL )
         {
@@ -285,12 +345,34 @@ void ExtcapOptionsDialog::updateWidgets()
 
         ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(allowStart);
 
-        ui->verticalLayout->addLayout(layout);
+        QWidget * mainWidget = Q_NULLPTR;
+
+        /* We should never display the dialog, if no settings are present */
+        Q_ASSERT(layouts.count() > 0);
+
+        if ( layouts.count() > 1 )
+        {
+            QTabWidget * tabs = new QTabWidget(this);
+            foreach ( QString key, groupKeys )
+            {
+                layouts[key]->layout()->addItem(new QSpacerItem(0, 0, QSizePolicy::Minimum, QSizePolicy::MinimumExpanding));
+                tabs->addTab(layouts[key], key);
+            }
+
+            tabs->setCurrentIndex(0);
+            mainWidget = tabs;
+        }
+        else if ( layouts.count() == 1 )
+            mainWidget = layouts[layouts.keys().at(0)];
+
+        ui->verticalLayout->addWidget(mainWidget);
         ui->verticalLayout->addSpacerItem(new QSpacerItem(20, 100, QSizePolicy::Minimum, QSizePolicy::Expanding));
     }
     else
     {
-        delete layout;
+        QList<QString> keys = layouts.keys();
+        foreach ( QString key, keys )
+            delete(layouts[key]);
     }
 }
 
@@ -382,45 +464,74 @@ void ExtcapOptionsDialog::resetValues()
 {
     ExtcapArgumentList::const_iterator iter;
     QString value;
+    bool doStore = false;
 
-    if (ui->verticalLayout->children().count() > 0)
+    int count = ui->verticalLayout->count();
+    if (count > 0)
     {
-        QGridLayout * layout = (QGridLayout *)ui->verticalLayout->findChild<QGridLayout *>();
+        QList<QLayout *> layouts;
+        if ( qobject_cast<QTabWidget *>(ui->verticalLayout->itemAt(0)->widget()) )
+        {
+            QTabWidget * tabs = qobject_cast<QTabWidget *>(ui->verticalLayout->itemAt(0)->widget());
+            for ( int cnt = 0; cnt < tabs->count(); cnt++ )
+            {
+                layouts.append(tabs->widget(cnt)->layout());
+            }
+        }
+        else
+            layouts.append(ui->verticalLayout->itemAt(0)->layout());
 
-        for ( int row = 0; row < layout->rowCount(); row++ )
+        for ( int cnt = 0; cnt < layouts.count(); cnt++ )
         {
-            QWidget * child = layout->itemAtPosition(row, 1)->widget();
+            QGridLayout * layout = qobject_cast<QGridLayout *>(layouts.at(cnt));
+            if ( ! layout )
+                continue;
 
-            if ( child )
+            for ( int row = 0; row < layout->rowCount(); row++ )
             {
-                /* Don't need labels, the edit widget contains the extcapargument property value */
-                ExtcapArgument * arg = 0;
-                QVariant prop = child->property(QString("extcap").toLocal8Bit());
+                QWidget * child = Q_NULLPTR;
+                if ( layout->itemAtPosition(row, 1) )
+                    child = qobject_cast<QWidget *>(layout->itemAtPosition(row, 1)->widget());
 
-                if ( prop.isValid() )
+                if ( child )
                 {
-                    arg = VariantPointer<ExtcapArgument>::asPtr(prop);
+                    /* Don't need labels, the edit widget contains the extcapargument property value */
+                    ExtcapArgument * arg = 0;
+                    QVariant prop = child->property(QString("extcap").toLocal8Bit());
 
-                    /* value<> can fail */
-                    if (arg)
+                    if ( prop.isValid() )
                     {
-                        arg->resetValue();
+                        arg = VariantPointer<ExtcapArgument>::asPtr(prop);
 
-                        /* replacing the edit widget after resetting will lead to default value */
-                        layout->removeItem(layout->itemAtPosition(row, 1));
-                        QWidget * editWidget = arg->createEditor((QWidget *) this);
-                        if ( editWidget != NULL )
+                        /* value<> can fail */
+                        if (arg)
                         {
-                            editWidget->setProperty(QString("extcap").toLocal8Bit(), VariantPointer<ExtcapArgument>::asQVariant(arg));
-                            layout->addWidget(editWidget, row, 1, Qt::AlignVCenter);
+                            arg->resetValue();
+
+                            /* replacing the edit widget after resetting will lead to default value */
+                            QWidget * newWidget = arg->createEditor((QWidget *) this);
+                            if ( newWidget != NULL )
+                            {
+                                newWidget->setProperty(QString("extcap").toLocal8Bit(), VariantPointer<ExtcapArgument>::asQVariant(arg));
+                                QLayoutItem * oldItem = layout->replaceWidget(child, newWidget);
+                                if ( oldItem )
+                                {
+                                    delete child;
+                                    delete oldItem;
+                                }
+                            }
+
+                            doStore = true;
                         }
                     }
                 }
             }
+
         }
 
         /* this stores all values to the preferences */
-        storeValues();
+        if ( doStore )
+            storeValues();
     }
 }