Utils: Make Layouting::Group { .. } less weird

It had a implicit vertical layout leading to unneded layout nesting
in quite a few cases. The price is an added Column { ... } in those
places where the implicit vertical layout was sufficient before.

Change-Id: I3ae1f03f9c1d691bd0c563b0447edd03ee02bbd2
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

src/libs/utils/layoutbuilder.cpp

@@ -25,7 +25,6 @@
 
 #include "layoutbuilder.h"
 
-#include "algorithm.h"
 #include "aspects.h"
 #include "qtcassert.h"
 
@@ -287,7 +286,6 @@ LayoutBuilder::LayoutItem::LayoutItem(const LayoutBuilder &builder)
 {
     layout = builder.createLayout();
     doLayoutHelper(layout, builder.m_items);
-    setMargins(builder.m_withMargins, layout);
 }
 
 /*!
@@ -386,13 +384,13 @@ LayoutBuilder &LayoutBuilder::addItem(const LayoutItem &item)
     return *this;
 }
 
-void LayoutBuilder::doLayout(QWidget *parent)
+void LayoutBuilder::doLayout(QWidget *parent, bool withMargins) const
 {
     QLayout *layout = createLayout();
     parent->setLayout(layout);
 
     doLayoutHelper(layout, m_items);
-    setMargins(m_withMargins, layout);
+    setMargins(withMargins, layout);
 }
 
 /*!
@@ -410,17 +408,15 @@ LayoutBuilder &LayoutBuilder::addItems(const LayoutItems &items)
 
     This operation can only be performed once per LayoutBuilder instance.
  */
-void LayoutBuilder::attachTo(QWidget *w, bool withMargins)
+void LayoutBuilder::attachTo(QWidget *w, bool withMargins) const
 {
-    m_withMargins = withMargins;
+    doLayout(w, withMargins);
-    doLayout(w);
 }
 
 QWidget *LayoutBuilder::emerge(bool withMargins)
 {
-    m_withMargins = withMargins;
     auto w = new QWidget;
-    doLayout(w);
+    doLayout(w, withMargins);
     return w;
 }
 
@@ -485,25 +481,23 @@ LayoutBuilder::AlignAsFormLabel::AlignAsFormLabel(const LayoutItem &item)
 
 namespace Layouting {
 
-Group::Group(std::initializer_list<LayoutItem> items)
+Group::Group(const LayoutBuilder &innerLayout)
+    : Group(Title({}), innerLayout)
+{}
+
+Group::Group(const LayoutBuilder::Title &title, const LayoutBuilder &innerLayout)
 {
     auto box = new QGroupBox;
-    Column builder;
+
-    bool innerMargins = true;
+    box->setTitle(title.specialValue.toString());
-    for (const LayoutItem &item : items) {
+    box->setObjectName(title.specialValue.toString());
-        if (item.specialType == LayoutBuilder::SpecialType::Title) {
+    if (auto check = qobject_cast<BoolAspect *>(title.aspect)) {
-            box->setTitle(item.specialValue.toString());
-            box->setObjectName(item.specialValue.toString());
-            if (auto check = qobject_cast<BoolAspect *>(item.aspect)) {
         box->setCheckable(true);
         box->setChecked(check->value());
         check->setHandlesGroup(box);
     }
-        } else {
+
-            builder.addItem(item);
+    innerLayout.attachTo(box, true);
-        }
-    }
-    builder.attachTo(box, innerMargins);
     widget = box;
 }
 

src/libs/utils/layoutbuilder.h

@@ -110,7 +110,7 @@ public:
 
     LayoutType layoutType() const { return m_layoutType; }
 
-    void attachTo(QWidget *w, bool withMargins = true);
+    void attachTo(QWidget *w, bool withMargins = true) const;
     QWidget *emerge(bool withMargins = true);
 
     class QTCREATOR_UTILS_EXPORT Space : public LayoutItem
@@ -153,12 +153,11 @@ protected:
     explicit LayoutBuilder(); // Adds to existing layout.
 
     QLayout *createLayout() const;
-    void doLayout(QWidget *parent);
+    void doLayout(QWidget *parent, bool withMargins) const;
 
     LayoutItems m_items;
     LayoutType m_layoutType;
     Utils::optional<int> m_spacing;
-    bool m_withMargins = false;
 };
 
 class QTCREATOR_UTILS_EXPORT LayoutExtender : public LayoutBuilder
@@ -176,7 +175,8 @@ namespace Layouting {
 class QTCREATOR_UTILS_EXPORT Group : public LayoutBuilder::LayoutItem
 {
 public:
-    Group(std::initializer_list<LayoutBuilder::LayoutItem> items);
+    explicit Group(const LayoutBuilder &innerLayout);
+    Group(const LayoutBuilder::Title &title, const LayoutBuilder &innerLayout);
 };
 
 class QTCREATOR_UTILS_EXPORT Column : public LayoutBuilder

src/plugins/autotest/ctest/ctestsettings.cpp

@@ -155,14 +155,16 @@ CTestSettingsPage::CTestSettingsPage(CTestSettings *settings, Utils::Id settings
             Row {s.stopOnFailure}, nl,
             Row {s.outputMode}, nl,
             Group {
-                Title(tr("Repeat tests"), &s.repeat), nl,
+                Title(tr("Repeat tests"), &s.repeat),
                 Row {s.repetitionMode, s.repetitionCount},
             }, nl,
             Group {
-                Title(tr("Run in parallel"), &s.parallel), nl,
+                Title(tr("Run in parallel"), &s.parallel),
+                Column {
                     Row {s.jobs}, nl,
                     Row {s.testLoad, s.threshold}
                 }
+            }
         };
 
         Column {Row {Column { form , Stretch() }, Stretch() } }.attachTo(widget);

src/plugins/autotest/qtest/qttestsettings.cpp

@@ -134,7 +134,7 @@ QtTestSettingsPage::QtTestSettingsPage(QtTestSettings *settings, Id settingsId)
             },
             Group {
                 Title(QtTestSettings::tr("Benchmark Metrics")),
-                s.metrics
+                Column { s.metrics }
             },
         };
 

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp

@@ -347,6 +347,7 @@ CMakeBuildSettingsWidget::CMakeBuildSettingsWidget(CMakeBuildSystem *bs) :
         Column {
             m_configurationStates,
             Group {
+                Column {
                     cmakeConfiguration,
                     Row {
                         bc->aspect<InitialCMakeArgumentsAspect>(),
@@ -354,6 +355,7 @@ CMakeBuildSettingsWidget::CMakeBuildSettingsWidget(CMakeBuildSystem *bs) :
                     },
                     m_reconfigureButton,
                 }
+            }
         }.setSpacing(0)
     }.attachTo(details, false);
 

src/plugins/cmakeprojectmanager/cmakespecificsettings.cpp

@@ -97,7 +97,7 @@ CMakeSpecificSettingsPage::CMakeSpecificSettingsPage(CMakeSpecificSettings *sett
         Column {
             Group {
                 Title(::CMakeProjectManager::Internal::CMakeSpecificSettings::tr("Adding Files")),
-                s.afterAddFileSetting
+                Column { s.afterAddFileSetting }
             },
             s.packageManagerAutoSetup,
             s.askBeforeReConfigureInitialParams,

src/plugins/coreplugin/systemsettings.cpp

@@ -207,7 +207,12 @@ public:
         form.addRow(Span(3, Row{m_clearCrashReportsButton, m_crashReportsSizeText, Stretch()}));
 #endif
 
-        Column{Group{Title(tr("System")), form, Stretch()}}.attachTo(this);
+        Column {
+            Group {
+                Title(tr("System")),
+                Column { form, Stretch() }
+            }
+        }.attachTo(this);
 
         m_reloadBehavior->setCurrentIndex(EditorManager::reloadSetting());
         if (HostOsInfo::isAnyUnixHost()) {

src/plugins/cvs/cvssettings.cpp

@@ -112,12 +112,14 @@ CvsSettingsPage::CvsSettingsPage(CvsSettings *settings)
             },
             Group {
                 Title(CvsSettings::tr("Miscellaneous")),
+                Column {
                     Form {
                         s.timeout,
                         s.diffOptions,
                     },
                     s.promptOnSubmit,
                     s.describeByCommitId,
+                }
             },
             Stretch()
         }.attachTo(widget);

src/plugins/debugger/cdb/cdboptionspage.cpp

@@ -199,29 +199,35 @@ CdbOptionsPageWidget::CdbOptionsPageWidget()
         Row {
             Group {
                 Title(Tr::tr("Startup")),
+                Column {
                     s.cdbAdditionalArguments,
                     s.useCdbConsole,
                     Stretch()
+                 }
             },
 
             Group {
                 Title(Tr::tr("Various")),
+                Column {
                     s.ignoreFirstChanceAccessViolation,
                     s.cdbBreakOnCrtDbgReport,
                     s.cdbBreakPointCorrection,
                     s.cdbUsePythonDumper
                 }
+            }
         },
 
         Group {
             Title(Tr::tr("Break On")),
-            m_breakEventWidget
+            Column { m_breakEventWidget }
         },
 
         Group {
             Title(Tr::tr("Add Exceptions to Issues View")),
+            Column {
                 s.firstChanceExceptionTaskEntry,
                 s.secondChanceExceptionTaskEntry
+            }
         },
 
         Stretch()
@@ -275,8 +281,8 @@ CdbPathsPageWidget::CdbPathsPageWidget()
 
     finish();
     Column {
-        Group { Title(Tr::tr("Symbol Paths")), m_symbolPaths },
+        Group { Title(Tr::tr("Symbol Paths")), Column { m_symbolPaths } },
-        Group { Title(Tr::tr("Source Paths")), m_sourcePaths },
+        Group { Title(Tr::tr("Source Paths")), Column { m_sourcePaths } },
         Stretch()
     }.attachTo(this);
 }

src/plugins/debugger/commonoptionspage.cpp

@@ -160,13 +160,16 @@ public:
             label,
             s.useCodeModel,
             s.showThreadNames,
-            Group { Title(Tr::tr("Extra Debugging Helper")), s.extraDumperFile }
+            Group { Title(Tr::tr("Extra Debugging Helper")), Column { s.extraDumperFile } }
         };
 
         Group useHelper {
             Row {
                 left,
-                Group { Title(Tr::tr("Debugging Helper Customization")), s.extraDumperCommands }
+                Group {
+                    Title(Tr::tr("Debugging Helper Customization")),
+                    Column { s.extraDumperCommands }
+                }
             }
         };
 

src/plugins/debugger/gdb/gdboptionspage.cpp

@@ -60,6 +60,7 @@ public:
 
             Group general {
                 Title { Tr::tr("General") },
+                Column {
                     Row { s.gdbWatchdogTimeout, Stretch() },
                     s.skipKnownFrames,
                     s.useMessageBoxForSignals,
@@ -71,11 +72,18 @@ public:
                     s.usePseudoTracepoints,
                     s.useIndexCache,
                     Stretch()
+                 }
             };
 
             Column commands {
-                Group { Title { Tr::tr("Additional Startup Commands") }, s.gdbStartupCommands },
+                Group {
-                Group { Title { Tr::tr("Additional Attach Commands") }, s.gdbPostAttachCommands },
+                    Title { Tr::tr("Additional Startup Commands") },
+                    Column { s.gdbStartupCommands }
+                },
+                Group {
+                    Title { Tr::tr("Additional Attach Commands") },
+                    Column { s.gdbPostAttachCommands },
+                },
                 Stretch()
             };
 
@@ -112,6 +120,7 @@ public:
 
             Group extended {
                 Title(Tr::tr("Extended")),
+                Column {
                     labelDangerous,
                     s.targetAsync,
                     s.autoEnrichParameters,
@@ -120,6 +129,7 @@ public:
                     s.breakOnAbort,
                     s.enableReverseDebugging,
                     s.multiInferior,
+                }
             };
 
             Column { extended, Stretch() }.attachTo(w);

src/plugins/fakevim/fakevimplugin.cpp

@@ -429,18 +429,22 @@ void FakeVimOptionPage::layoutPage(QWidget *widget)
 
         Group {
             Title(Tr::tr("Vim Behavior")),
+            Column {
                 bools,
                 ints,
                 strings
+            }
         },
 
         Group {
             Title(Tr::tr("Plugin Emulation")),
+            Column {
                 s.emulateVimCommentary,
                 s.emulateReplaceWithRegister,
                 s.emulateArgTextObj,
                 s.emulateExchange,
                 s.emulateSurround
+            }
         },
 
         Row { copyTextEditorSettings, setQtStyle, setPlainStyle, Stretch() },

src/plugins/git/gitsettings.cpp

@@ -170,14 +170,18 @@ GitSettingsPage::GitSettingsPage(GitSettings *settings)
         Column {
             Group {
                 Title(GitSettings::tr("Configuration")),
+                Column {
                     Row { s.path },
                     s.winSetHomeEnvironment,
+                }
             },
 
             Group {
                 Title(GitSettings::tr("Miscellaneous")),
+                Column {
                     Row { s.logCount, s.timeout, Stretch() },
                     s.pullRebase
+                }
             },
 
             Group {

src/plugins/gitlab/gitlaboptionspage.cpp

@@ -148,7 +148,7 @@ GitLabOptionsWidget::GitLabOptionsWidget(QWidget *parent)
     Grid {
         Form {
             defaultLabel, m_defaultGitLabServer, nl,
-            Row { Group { m_gitLabServerWidget, Space(1) } }, nl,
+            Row { Group { Column { m_gitLabServerWidget, Space(1) } } }, nl,
             m_curl, nl,
         }, Column { m_add, m_edit, m_remove, Stretch() },
     }.attachTo(this);

src/plugins/nim/settings/nimsettings.cpp

@@ -143,7 +143,7 @@ NimToolsSettingsPage::NimToolsSettingsPage(NimSettings *settings)
         Column {
             Group {
                 Title("Nimsuggest"),
-                settings->nimSuggestPath
+                Column { settings->nimSuggestPath }
             },
             Stretch()
         }.attachTo(widget);

src/plugins/perforce/perforcesettings.cpp

@@ -277,9 +277,11 @@ PerforceSettingsPage::PerforceSettingsPage(PerforceSettings *settings)
 
         Group misc {
             Title(PerforceSettings::tr("Miscellaneous")),
+            Column {
                 Row { s.logCount, s.timeOutS, Stretch() },
                 s.promptToSubmit,
                 s.autoOpen
+            }
         };
 
         Column {

src/plugins/subversion/subversionsettings.cpp

@@ -122,7 +122,7 @@ SubversionSettingsPage::SubversionSettingsPage(SubversionSettings *settings)
         Column {
             Group {
                 Title(SubversionSettings::tr("Configuration")),
-                s.binaryPath
+                Column { s.binaryPath }
             },
 
             Group {
@@ -135,9 +135,11 @@ SubversionSettingsPage::SubversionSettingsPage(SubversionSettings *settings)
 
             Group {
                 Title(SubversionSettings::tr("Miscellaneous")),
+                Column {
                     Row { s.logCount, s.timeout, Stretch() },
                     s.promptOnSubmit,
                     s.spaceIgnorantAnnotation,
+                }
             },
 
             Stretch()

src/plugins/todo/keyworddialog.cpp

@@ -81,7 +81,7 @@ KeywordDialog::KeywordDialog(const Keyword &keyword, const QSet<QString> &alread
             },
             Group {
                 Title(Tr::tr("Keyword")),
-                m_keywordNameEdit
+                Column { m_keywordNameEdit }
             }
         },
         m_errorLabel,

src/plugins/valgrind/valgrindconfigwidget.cpp

@@ -89,12 +89,14 @@ ValgrindConfigWidget::ValgrindConfigWidget(ValgrindBaseSettings *settings)
         Span {
             2,
             Group {
+                Column {
                     s.enableCacheSim,
                     s.enableBranchSim,
                     s.collectSystime,
                     s.collectBusEvents,
                 }
             }
+        }
     };
 
     Column {