Utils: Don't store widgets in StringAspect

Change-Id: Ibeabd5c933210fefede7748cdfba416ed33b27bf
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/libs/utils/aspects.cpp

@@ -834,10 +834,6 @@ public:
     Qt::TextElideMode m_elideMode = Qt::ElideNone;
     QString m_placeHolderText;
     Key m_historyCompleterKey;
-    QPointer<ElidingLabel> m_labelDisplay;
-    QPointer<FancyLineEdit> m_lineEditDisplay;
-    QPointer<ShowPasswordButton> m_showPasswordButton;
-    QPointer<QTextEdit> m_textEditDisplay;
     MacroExpanderProvider m_expanderProvider;
     StringAspect::ValueAcceptor m_valueAcceptor;
     std::optional<FancyLineEdit::ValidationFunction> m_validator;
@@ -850,6 +846,8 @@ public:
     bool m_useResetButton = false;
     bool m_autoApplyOnEditingFinished = false;
     bool m_validatePlaceHolder = false;
+
+    UndoableValue<QString> undoable;
 };
 
 class IntegerAspectPrivate
@@ -1031,11 +1029,11 @@ void StringAspect::setDisplayStyle(DisplayStyle displayStyle)
 */
 void StringAspect::setPlaceHolderText(const QString &placeHolderText)
 {
+    if (d->m_placeHolderText == placeHolderText)
+        return;
+
     d->m_placeHolderText = placeHolderText;
-    if (d->m_lineEditDisplay)
+    emit placeholderTextChanged(placeHolderText);
-        d->m_lineEditDisplay->setPlaceholderText(placeHolderText);
-    if (d->m_textEditDisplay)
-        d->m_textEditDisplay->setPlaceholderText(placeHolderText);
 }
 
 /*!
@@ -1043,9 +1041,10 @@ void StringAspect::setPlaceHolderText(const QString &placeHolderText)
 */
 void StringAspect::setElideMode(Qt::TextElideMode elideMode)
 {
+    if (d->m_elideMode == elideMode)
+        return;
     d->m_elideMode = elideMode;
-    if (d->m_labelDisplay)
+    emit elideModeChanged(elideMode);
-        d->m_labelDisplay->setElideMode(elideMode);
 }
 
 /*!
@@ -1057,22 +1056,13 @@ void StringAspect::setElideMode(Qt::TextElideMode elideMode)
 void StringAspect::setHistoryCompleter(const Key &historyCompleterKey)
 {
     d->m_historyCompleterKey = historyCompleterKey;
-    if (d->m_lineEditDisplay)
+    emit historyCompleterKeyChanged(historyCompleterKey);
-        d->m_lineEditDisplay->setHistoryCompleter(historyCompleterKey);
-}
-
-void StringAspect::setUndoRedoEnabled(bool undoRedoEnabled)
-{
-    d->m_undoRedoEnabled = undoRedoEnabled;
-    if (d->m_textEditDisplay)
-        d->m_textEditDisplay->setUndoRedoEnabled(undoRedoEnabled);
 }
 
 void StringAspect::setAcceptRichText(bool acceptRichText)
 {
     d->m_acceptRichText = acceptRichText;
-    if (d->m_textEditDisplay)
+    emit acceptRichTextChanged(acceptRichText);
-        d->m_textEditDisplay->setAcceptRichText(acceptRichText);
 }
 
 void StringAspect::setMacroExpanderProvider(const MacroExpanderProvider &expanderProvider)
@@ -1093,8 +1083,7 @@ void StringAspect::setUseResetButton()
 void StringAspect::setValidationFunction(const FancyLineEdit::ValidationFunction &validator)
 {
     d->m_validator = validator;
-    if (d->m_lineEditDisplay)
+    emit validationFunctionChanged(validator);
-        d->m_lineEditDisplay->setValidationFunction(*d->m_validator);
 }
 
 void StringAspect::setAutoApplyOnEditingFinished(bool applyOnEditingFinished)
@@ -1102,12 +1091,6 @@ void StringAspect::setAutoApplyOnEditingFinished(bool applyOnEditingFinished)
     d->m_autoApplyOnEditingFinished = applyOnEditingFinished;
 }
 
-void StringAspect::validateInput()
-{
-    if (d->m_lineEditDisplay)
-        d->m_lineEditDisplay->validate();
-}
-
 void StringAspect::addToLayout(LayoutItem &parent)
 {
     d->m_checkerImpl.addToLayoutFirst(parent);
@@ -1124,78 +1107,150 @@ void StringAspect::addToLayout(LayoutItem &parent)
 
     switch (d->m_displayStyle) {
     case PasswordLineEditDisplay:
-    case LineEditDisplay:
+    case LineEditDisplay: {
-        d->m_lineEditDisplay = createSubWidget<FancyLineEdit>();
+        auto lineEditDisplay = createSubWidget<FancyLineEdit>();
-        d->m_lineEditDisplay->setPlaceholderText(d->m_placeHolderText);
+        lineEditDisplay->setPlaceholderText(d->m_placeHolderText);
         if (!d->m_historyCompleterKey.isEmpty())
-            d->m_lineEditDisplay->setHistoryCompleter(d->m_historyCompleterKey);
+            lineEditDisplay->setHistoryCompleter(d->m_historyCompleterKey);
+
+        connect(this,
+                &StringAspect::historyCompleterKeyChanged,
+                lineEditDisplay,
+                [lineEditDisplay](const Key &historyCompleterKey) {
+                    lineEditDisplay->setHistoryCompleter(historyCompleterKey);
+                });
+        connect(this,
+                &StringAspect::placeholderTextChanged,
+                lineEditDisplay,
+                &FancyLineEdit::setPlaceholderText);
 
         if (d->m_validator)
-            d->m_lineEditDisplay->setValidationFunction(*d->m_validator);
+            lineEditDisplay->setValidationFunction(*d->m_validator);
-        d->m_lineEditDisplay->setTextKeepingActiveCursor(displayedString);
+        lineEditDisplay->setTextKeepingActiveCursor(displayedString);
-        d->m_lineEditDisplay->setReadOnly(isReadOnly());
+        lineEditDisplay->setReadOnly(isReadOnly());
-        d->m_lineEditDisplay->setValidatePlaceHolder(d->m_validatePlaceHolder);
+        lineEditDisplay->setValidatePlaceHolder(d->m_validatePlaceHolder);
-        d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_lineEditDisplay.data());
+
-        addLabeledItem(parent, d->m_lineEditDisplay);
+        d->m_checkerImpl.updateWidgetFromCheckStatus(this, lineEditDisplay);
-        useMacroExpander(d->m_lineEditDisplay);
+        connect(d->m_checkerImpl.m_checked.get(),
+                &BoolAspect::volatileValueChanged,
+                lineEditDisplay,
+                [this, lineEditDisplay]() {
+                    d->m_checkerImpl.updateWidgetFromCheckStatus(this, lineEditDisplay);
+                });
+
+        addLabeledItem(parent, lineEditDisplay);
+        useMacroExpander(lineEditDisplay);
         if (d->m_useResetButton) {
             auto resetButton = createSubWidget<QPushButton>(Tr::tr("Reset"));
-            resetButton->setEnabled(d->m_lineEditDisplay->text() != defaultValue());
+            resetButton->setEnabled(lineEditDisplay->text() != defaultValue());
-            connect(resetButton, &QPushButton::clicked, this, [this] {
+            connect(resetButton, &QPushButton::clicked, lineEditDisplay, [this, lineEditDisplay] {
-                d->m_lineEditDisplay->setText(defaultValue());
+                lineEditDisplay->setText(defaultValue());
             });
-            connect(d->m_lineEditDisplay, &QLineEdit::textChanged, this, [this, resetButton] {
+            connect(lineEditDisplay,
-                resetButton->setEnabled(d->m_lineEditDisplay->text() != defaultValue());
+                    &QLineEdit::textChanged,
+                    resetButton,
+                    [this, lineEditDisplay, resetButton] {
+                        resetButton->setEnabled(lineEditDisplay->text() != defaultValue());
                     });
             parent.addItem(resetButton);
         }
-        connect(d->m_lineEditDisplay, &FancyLineEdit::validChanged, this, &StringAspect::validChanged);
+        connect(lineEditDisplay, &FancyLineEdit::validChanged, this, &StringAspect::validChanged);
         bufferToGui();
         if (isAutoApply() && d->m_autoApplyOnEditingFinished) {
-            connect(d->m_lineEditDisplay, &FancyLineEdit::editingFinished,
+            connect(lineEditDisplay,
-                    this, &StringAspect::handleGuiChanged);
+                    &FancyLineEdit::editingFinished,
+                    this,
+                    [this, lineEditDisplay]() {
+                        d->undoable.set(undoStack(), lineEditDisplay->text());
+                        handleGuiChanged();
+                    });
         } else {
-            connect(d->m_lineEditDisplay, &QLineEdit::textEdited,
+            connect(lineEditDisplay, &QLineEdit::textEdited, this, [this, lineEditDisplay]() {
-                    this, &StringAspect::handleGuiChanged);
+                d->undoable.set(undoStack(), lineEditDisplay->text());
+                handleGuiChanged();
+            });
         }
         if (d->m_displayStyle == PasswordLineEditDisplay) {
-            d->m_showPasswordButton = createSubWidget<ShowPasswordButton>();
+            auto showPasswordButton = createSubWidget<ShowPasswordButton>();
-            d->m_lineEditDisplay->setEchoMode(QLineEdit::PasswordEchoOnEdit);
+            lineEditDisplay->setEchoMode(QLineEdit::PasswordEchoOnEdit);
-            parent.addItem(d->m_showPasswordButton);
+            parent.addItem(showPasswordButton);
-            connect(d->m_showPasswordButton,
+            connect(showPasswordButton,
                     &ShowPasswordButton::toggled,
-                    d->m_lineEditDisplay,
+                    lineEditDisplay,
-                    [this] {
+                    [showPasswordButton, lineEditDisplay] {
-                        d->m_lineEditDisplay->setEchoMode(d->m_showPasswordButton->isChecked()
+                        lineEditDisplay->setEchoMode(showPasswordButton->isChecked()
                                                          ? QLineEdit::Normal
                                                          : QLineEdit::PasswordEchoOnEdit);
                     });
         }
+
+        connect(&d->undoable.m_signal,
+                &UndoSignaller::changed,
+                lineEditDisplay,
+                [this, lineEditDisplay] {
+                    lineEditDisplay->setTextKeepingActiveCursor(d->undoable.get());
+                    lineEditDisplay->validate();
+                });
+
         break;
-    case TextEditDisplay:
+    }
-        d->m_textEditDisplay = createSubWidget<QTextEdit>();
+    case TextEditDisplay: {
-        d->m_textEditDisplay->setPlaceholderText(d->m_placeHolderText);
+        auto textEditDisplay = createSubWidget<QTextEdit>();
-        d->m_textEditDisplay->setUndoRedoEnabled(d->m_undoRedoEnabled);
+        textEditDisplay->setPlaceholderText(d->m_placeHolderText);
-        d->m_textEditDisplay->setAcceptRichText(d->m_acceptRichText);
+        textEditDisplay->setUndoRedoEnabled(false);
-        d->m_textEditDisplay->setTextInteractionFlags(Qt::TextEditorInteraction);
+        textEditDisplay->setAcceptRichText(d->m_acceptRichText);
-        d->m_textEditDisplay->setText(displayedString);
+        textEditDisplay->setTextInteractionFlags(Qt::TextEditorInteraction);
-        d->m_textEditDisplay->setReadOnly(isReadOnly());
+        textEditDisplay->setText(displayedString);
-        d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_textEditDisplay.data());
+        textEditDisplay->setReadOnly(isReadOnly());
-        addLabeledItem(parent, d->m_textEditDisplay);
+        d->m_checkerImpl.updateWidgetFromCheckStatus(this, textEditDisplay);
-        useMacroExpander(d->m_textEditDisplay);
+
+        connect(d->m_checkerImpl.m_checked.get(),
+                &BoolAspect::volatileValueChanged,
+                textEditDisplay,
+                [this, textEditDisplay]() {
+                    d->m_checkerImpl.updateWidgetFromCheckStatus(this, textEditDisplay);
+                });
+
+        addLabeledItem(parent, textEditDisplay);
+        useMacroExpander(textEditDisplay);
         bufferToGui();
-        connect(d->m_textEditDisplay, &QTextEdit::textChanged,
+        connect(this,
-                this, &StringAspect::handleGuiChanged);
+                &StringAspect::acceptRichTextChanged,
+                textEditDisplay,
+                &QTextEdit::setAcceptRichText);
+        connect(this,
+                &StringAspect::placeholderTextChanged,
+                textEditDisplay,
+                &QTextEdit::setPlaceholderText);
+
+        connect(textEditDisplay, &QTextEdit::textChanged, this, [this, textEditDisplay]() {
+            d->undoable.set(undoStack(), textEditDisplay->toPlainText());
+            handleGuiChanged();
+        });
+
+        connect(&d->undoable.m_signal,
+                &UndoSignaller::changed,
+                textEditDisplay,
+                [this, textEditDisplay] { textEditDisplay->setText(d->undoable.get()); });
         break;
-    case LabelDisplay:
+    }
-        d->m_labelDisplay = createSubWidget<ElidingLabel>();
+    case LabelDisplay: {
-        d->m_labelDisplay->setElideMode(d->m_elideMode);
+        auto labelDisplay = createSubWidget<ElidingLabel>();
-        d->m_labelDisplay->setTextInteractionFlags(Qt::TextSelectableByMouse);
+        labelDisplay->setElideMode(d->m_elideMode);
-        d->m_labelDisplay->setText(displayedString);
+        labelDisplay->setTextInteractionFlags(Qt::TextSelectableByMouse);
-        d->m_labelDisplay->setToolTip(d->m_showToolTipOnLabel ? displayedString : toolTip());
+        labelDisplay->setText(displayedString);
-        addLabeledItem(parent, d->m_labelDisplay);
+        labelDisplay->setToolTip(d->m_showToolTipOnLabel ? displayedString : toolTip());
+        connect(this, &StringAspect::setElideMode, labelDisplay, &ElidingLabel::setElideMode);
+        addLabeledItem(parent, labelDisplay);
+
+        connect(&d->undoable.m_signal, &UndoSignaller::changed, labelDisplay, [this, labelDisplay] {
+            labelDisplay->setText(d->undoable.get());
+            labelDisplay->setToolTip(d->m_showToolTipOnLabel ? d->undoable.get() : toolTip());
+        });
+
         break;
     }
+    }
 
     d->m_checkerImpl.addToLayoutLast(parent);
 }
@@ -1208,11 +1263,7 @@ QString StringAspect::expandedValue() const
 
 bool StringAspect::guiToBuffer()
 {
-    if (d->m_lineEditDisplay)
+    return updateStorage(m_buffer, d->undoable.get());
-        return updateStorage(m_buffer, d->m_lineEditDisplay->text());
-    if (d->m_textEditDisplay)
-        return updateStorage(m_buffer, d->m_textEditDisplay->document()->toPlainText());
-    return false;
 }
 
 bool StringAspect::bufferToInternal()
@@ -1232,24 +1283,7 @@ bool StringAspect::internalToBuffer()
 
 void StringAspect::bufferToGui()
 {
-    if (d->m_lineEditDisplay) {
+    d->undoable.setWithoutUndo(m_buffer);
-        d->m_lineEditDisplay->setTextKeepingActiveCursor(m_buffer);
-        d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_lineEditDisplay.data());
-    }
-
-    if (d->m_textEditDisplay) {
-        const QString old = d->m_textEditDisplay->document()->toPlainText();
-        if (m_buffer != old)
-            d->m_textEditDisplay->setText(m_buffer);
-        d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_textEditDisplay.data());
-    }
-
-    if (d->m_labelDisplay) {
-        d->m_labelDisplay->setText(m_buffer);
-        d->m_labelDisplay->setToolTip(d->m_showToolTipOnLabel ? m_buffer : toolTip());
-    }
-
-    validateInput();
 }
 
 /*!

src/libs/utils/aspects.h

@@ -562,7 +562,6 @@ public:
     void setDisplayFilter(const std::function<QString (const QString &)> &displayFilter);
     void setPlaceHolderText(const QString &placeHolderText);
     void setHistoryCompleter(const Key &historyCompleterKey);
-    void setUndoRedoEnabled(bool readOnly);
     void setAcceptRichText(bool acceptRichText);
     void setMacroExpanderProvider(const MacroExpanderProvider &expanderProvider);
     void setUseGlobalMacroExpander();
@@ -575,8 +574,6 @@ public:
     bool isChecked() const;
     void setChecked(bool checked);
 
-    void validateInput();
-
     enum DisplayStyle {
         LabelDisplay,
         LineEditDisplay,
@@ -591,6 +588,11 @@ public:
 
 signals:
     void validChanged(bool validState);
+    void elideModeChanged(Qt::TextElideMode elideMode);
+    void historyCompleterKeyChanged(const Key &historyCompleterKey);
+    void acceptRichTextChanged(bool acceptRichText);
+    void validationFunctionChanged(const FancyLineEdit::ValidationFunction &validator);
+    void placeholderTextChanged(const QString &placeholderText);
 
 protected:
     void bufferToGui() override;

src/plugins/qbsprojectmanager/qbsbuildstep.cpp

@@ -220,7 +220,6 @@ QbsBuildStep::QbsBuildStep(BuildStepList *bsl, Id id) :
 
     commandLine.setDisplayStyle(StringAspect::TextEditDisplay);
     commandLine.setLabelText(QbsProjectManager::Tr::tr("Equivalent command line:"));
-    commandLine.setUndoRedoEnabled(false);
     commandLine.setReadOnly(true);
 
     connect(&maxJobCount, &BaseAspect::changed, this, &QbsBuildStep::updateState);

src/plugins/qmakeprojectmanager/qmakestep.cpp

@@ -75,7 +75,6 @@ QMakeStep::QMakeStep(BuildStepList *bsl, Id id)
     effectiveCall.setDisplayStyle(StringAspect::TextEditDisplay);
     effectiveCall.setLabelText(Tr::tr("Effective qmake call:"));
     effectiveCall.setReadOnly(true);
-    effectiveCall.setUndoRedoEnabled(false);
     effectiveCall.setEnabled(true);
 
     auto updateSummary = [this] {