From b32643e9965c5016d7cddab67c24b8c10abf7ca9 Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Wed, 20 Sep 2023 08:21:40 +0200 Subject: [PATCH] Utils: Don't store widgets in StringAspect Change-Id: Ibeabd5c933210fefede7748cdfba416ed33b27bf Reviewed-by: hjk Reviewed-by: --- src/libs/utils/aspects.cpp | 240 ++++++++++-------- src/libs/utils/aspects.h | 8 +- .../qbsprojectmanager/qbsbuildstep.cpp | 1 - src/plugins/qmakeprojectmanager/qmakestep.cpp | 1 - 4 files changed, 142 insertions(+), 108 deletions(-) diff --git a/src/libs/utils/aspects.cpp b/src/libs/utils/aspects.cpp index c31d7b615e2..4b6062bfeb7 100644 --- a/src/libs/utils/aspects.cpp +++ b/src/libs/utils/aspects.cpp @@ -834,10 +834,6 @@ public: Qt::TextElideMode m_elideMode = Qt::ElideNone; QString m_placeHolderText; Key m_historyCompleterKey; - QPointer m_labelDisplay; - QPointer m_lineEditDisplay; - QPointer m_showPasswordButton; - QPointer m_textEditDisplay; MacroExpanderProvider m_expanderProvider; StringAspect::ValueAcceptor m_valueAcceptor; std::optional m_validator; @@ -850,6 +846,8 @@ public: bool m_useResetButton = false; bool m_autoApplyOnEditingFinished = false; bool m_validatePlaceHolder = false; + + UndoableValue 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) - d->m_lineEditDisplay->setPlaceholderText(placeHolderText); - if (d->m_textEditDisplay) - d->m_textEditDisplay->setPlaceholderText(placeHolderText); + emit placeholderTextChanged(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) - d->m_labelDisplay->setElideMode(elideMode); + emit elideModeChanged(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) - d->m_lineEditDisplay->setHistoryCompleter(historyCompleterKey); -} - -void StringAspect::setUndoRedoEnabled(bool undoRedoEnabled) -{ - d->m_undoRedoEnabled = undoRedoEnabled; - if (d->m_textEditDisplay) - d->m_textEditDisplay->setUndoRedoEnabled(undoRedoEnabled); + emit historyCompleterKeyChanged(historyCompleterKey); } void StringAspect::setAcceptRichText(bool acceptRichText) { d->m_acceptRichText = acceptRichText; - if (d->m_textEditDisplay) - d->m_textEditDisplay->setAcceptRichText(acceptRichText); + emit acceptRichTextChanged(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) - d->m_lineEditDisplay->setValidationFunction(*d->m_validator); + emit validationFunctionChanged(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: - d->m_lineEditDisplay = createSubWidget(); - d->m_lineEditDisplay->setPlaceholderText(d->m_placeHolderText); + case LineEditDisplay: { + auto lineEditDisplay = createSubWidget(); + 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); - d->m_lineEditDisplay->setTextKeepingActiveCursor(displayedString); - d->m_lineEditDisplay->setReadOnly(isReadOnly()); - d->m_lineEditDisplay->setValidatePlaceHolder(d->m_validatePlaceHolder); - d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_lineEditDisplay.data()); - addLabeledItem(parent, d->m_lineEditDisplay); - useMacroExpander(d->m_lineEditDisplay); + lineEditDisplay->setValidationFunction(*d->m_validator); + lineEditDisplay->setTextKeepingActiveCursor(displayedString); + lineEditDisplay->setReadOnly(isReadOnly()); + lineEditDisplay->setValidatePlaceHolder(d->m_validatePlaceHolder); + + d->m_checkerImpl.updateWidgetFromCheckStatus(this, 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(Tr::tr("Reset")); - resetButton->setEnabled(d->m_lineEditDisplay->text() != defaultValue()); - connect(resetButton, &QPushButton::clicked, this, [this] { - d->m_lineEditDisplay->setText(defaultValue()); - }); - connect(d->m_lineEditDisplay, &QLineEdit::textChanged, this, [this, resetButton] { - resetButton->setEnabled(d->m_lineEditDisplay->text() != defaultValue()); + resetButton->setEnabled(lineEditDisplay->text() != defaultValue()); + connect(resetButton, &QPushButton::clicked, lineEditDisplay, [this, lineEditDisplay] { + lineEditDisplay->setText(defaultValue()); }); + connect(lineEditDisplay, + &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, - this, &StringAspect::handleGuiChanged); + connect(lineEditDisplay, + &FancyLineEdit::editingFinished, + this, + [this, lineEditDisplay]() { + d->undoable.set(undoStack(), lineEditDisplay->text()); + handleGuiChanged(); + }); } else { - connect(d->m_lineEditDisplay, &QLineEdit::textEdited, - this, &StringAspect::handleGuiChanged); + connect(lineEditDisplay, &QLineEdit::textEdited, this, [this, lineEditDisplay]() { + d->undoable.set(undoStack(), lineEditDisplay->text()); + handleGuiChanged(); + }); } if (d->m_displayStyle == PasswordLineEditDisplay) { - d->m_showPasswordButton = createSubWidget(); - d->m_lineEditDisplay->setEchoMode(QLineEdit::PasswordEchoOnEdit); - parent.addItem(d->m_showPasswordButton); - connect(d->m_showPasswordButton, + auto showPasswordButton = createSubWidget(); + lineEditDisplay->setEchoMode(QLineEdit::PasswordEchoOnEdit); + parent.addItem(showPasswordButton); + connect(showPasswordButton, &ShowPasswordButton::toggled, - d->m_lineEditDisplay, - [this] { - d->m_lineEditDisplay->setEchoMode(d->m_showPasswordButton->isChecked() - ? QLineEdit::Normal - : QLineEdit::PasswordEchoOnEdit); + lineEditDisplay, + [showPasswordButton, lineEditDisplay] { + 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(); - d->m_textEditDisplay->setPlaceholderText(d->m_placeHolderText); - d->m_textEditDisplay->setUndoRedoEnabled(d->m_undoRedoEnabled); - d->m_textEditDisplay->setAcceptRichText(d->m_acceptRichText); - d->m_textEditDisplay->setTextInteractionFlags(Qt::TextEditorInteraction); - d->m_textEditDisplay->setText(displayedString); - d->m_textEditDisplay->setReadOnly(isReadOnly()); - d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_textEditDisplay.data()); - addLabeledItem(parent, d->m_textEditDisplay); - useMacroExpander(d->m_textEditDisplay); + } + case TextEditDisplay: { + auto textEditDisplay = createSubWidget(); + textEditDisplay->setPlaceholderText(d->m_placeHolderText); + textEditDisplay->setUndoRedoEnabled(false); + textEditDisplay->setAcceptRichText(d->m_acceptRichText); + textEditDisplay->setTextInteractionFlags(Qt::TextEditorInteraction); + textEditDisplay->setText(displayedString); + textEditDisplay->setReadOnly(isReadOnly()); + d->m_checkerImpl.updateWidgetFromCheckStatus(this, 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, - this, &StringAspect::handleGuiChanged); + connect(this, + &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(); - d->m_labelDisplay->setElideMode(d->m_elideMode); - d->m_labelDisplay->setTextInteractionFlags(Qt::TextSelectableByMouse); - d->m_labelDisplay->setText(displayedString); - d->m_labelDisplay->setToolTip(d->m_showToolTipOnLabel ? displayedString : toolTip()); - addLabeledItem(parent, d->m_labelDisplay); + } + case LabelDisplay: { + auto labelDisplay = createSubWidget(); + labelDisplay->setElideMode(d->m_elideMode); + labelDisplay->setTextInteractionFlags(Qt::TextSelectableByMouse); + labelDisplay->setText(displayedString); + 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->m_lineEditDisplay->text()); - if (d->m_textEditDisplay) - return updateStorage(m_buffer, d->m_textEditDisplay->document()->toPlainText()); - return false; + return updateStorage(m_buffer, d->undoable.get()); } bool StringAspect::bufferToInternal() @@ -1232,24 +1283,7 @@ bool StringAspect::internalToBuffer() void StringAspect::bufferToGui() { - if (d->m_lineEditDisplay) { - 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(); + d->undoable.setWithoutUndo(m_buffer); } /*! diff --git a/src/libs/utils/aspects.h b/src/libs/utils/aspects.h index cd589aa75fe..ccdaadcc631 100644 --- a/src/libs/utils/aspects.h +++ b/src/libs/utils/aspects.h @@ -562,7 +562,6 @@ public: void setDisplayFilter(const std::function &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; diff --git a/src/plugins/qbsprojectmanager/qbsbuildstep.cpp b/src/plugins/qbsprojectmanager/qbsbuildstep.cpp index de5ddba1bb7..7968a555bf9 100644 --- a/src/plugins/qbsprojectmanager/qbsbuildstep.cpp +++ b/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); diff --git a/src/plugins/qmakeprojectmanager/qmakestep.cpp b/src/plugins/qmakeprojectmanager/qmakestep.cpp index f0a10dac178..afeba2bc446 100644 --- a/src/plugins/qmakeprojectmanager/qmakestep.cpp +++ b/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] {