From 8ef85e481aa3095667be33b5db34d180cd18be07 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Tue, 16 Jan 2024 11:18:30 +0100 Subject: [PATCH] ProjectExplorer: Re-do the EnvironmentWidget Editing in the item view was cumbersome and did not generally allow one variable to refer to another, as there was no defined order on the assignments. Therefore, we make the item view read-only and promote the batch editing widget to be the standard input method. User changes are not sorted anymore, except in the summary text of the details widget. Fixes: QTCREATORBUG-28480 Change-Id: I225cf86fff7b001a57d663e1fd267a4645e695c4 Reviewed-by: Reviewed-by: Qt CI Bot Reviewed-by: hjk --- src/libs/utils/namevaluemodel.cpp | 7 +- src/libs/utils/namevaluesdialog.cpp | 100 +++++++++++--- src/libs/utils/namevaluesdialog.h | 30 ++++- .../projectexplorer/environmentwidget.cpp | 126 +++++++++--------- .../projectexplorer/environmentwidget.h | 6 +- src/plugins/projectexplorer/kitaspects.cpp | 3 +- 6 files changed, 180 insertions(+), 92 deletions(-) diff --git a/src/libs/utils/namevaluemodel.cpp b/src/libs/utils/namevaluemodel.cpp index 2bd9180f769..57327c4ebed 100644 --- a/src/libs/utils/namevaluemodel.cpp +++ b/src/libs/utils/namevaluemodel.cpp @@ -179,7 +179,7 @@ QVariant NameValueModel::data(const QModelIndex &index, int role) const Qt::ItemFlags NameValueModel::flags(const QModelIndex &index) const { Q_UNUSED(index) - return Qt::ItemIsSelectable | Qt::ItemIsEditable | Qt::ItemIsEnabled; + return Qt::ItemIsSelectable | Qt::ItemIsEnabled; } QVariant NameValueModel::headerData(int section, Qt::Orientation orientation, int role) const @@ -264,10 +264,7 @@ bool NameValueModel::setData(const QModelIndex &index, const QVariant &value, in QModelIndex NameValueModel::addVariable() { - //: Name when inserting a new variable - return addVariable(NameValueItem(Tr::tr(""), - //: Value when inserting a new variable - Tr::tr(""))); + return addVariable(NameValueItem("NEWVAR", "VALUE")); } QModelIndex NameValueModel::addVariable(const NameValueItem &item) diff --git a/src/libs/utils/namevaluesdialog.cpp b/src/libs/utils/namevaluesdialog.cpp index b79a078d6c3..4f9537655ef 100644 --- a/src/libs/utils/namevaluesdialog.cpp +++ b/src/libs/utils/namevaluesdialog.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include namespace Utils { @@ -34,20 +36,21 @@ static EnvironmentItems cleanUp(const EnvironmentItems &items) return uniqueItems; } -class NameValueItemsWidget : public QWidget +class TextEditHelper : public QPlainTextEdit { + Q_OBJECT public: - explicit NameValueItemsWidget(QWidget *parent = nullptr); + using QPlainTextEdit::QPlainTextEdit; - void setEnvironmentItems(const EnvironmentItems &items); - EnvironmentItems environmentItems() const; - - void setPlaceholderText(const QString &text); +signals: + void lostFocus(); private: - QPlainTextEdit *m_editor; + void focusOutEvent(QFocusEvent *) override { emit lostFocus(); } }; +} // namespace Internal + NameValueItemsWidget::NameValueItemsWidget(QWidget *parent) : QWidget(parent) { @@ -60,26 +63,41 @@ NameValueItemsWidget::NameValueItemsWidget(QWidget *parent) "To clear a variable, put its name on a line with nothing else on it.\n" "To disable a variable, prefix the line with \"#\"."); - m_editor = new QPlainTextEdit(this); + m_editor = new Internal::TextEditHelper(this); auto layout = new QVBoxLayout(this); layout->setContentsMargins(0, 0, 0, 0); layout->addWidget(m_editor); layout->addWidget(new QLabel(helpText, this)); + + const auto checkForItemChange = [this] { + const NameValueItems newItems = environmentItems(); + if (newItems != m_originalItems) { + m_originalItems = newItems; + emit userChangedItems(newItems); + } + }; + const auto timer = new QTimer(this); + timer->setSingleShot(true); + timer->setInterval(1000); + connect(m_editor, &QPlainTextEdit::textChanged, timer, qOverload<>(&QTimer::start)); + connect(timer, &QTimer::timeout, this, checkForItemChange); + connect(m_editor, &Internal::TextEditHelper::lostFocus, this, [timer, checkForItemChange] { + timer->stop(); + checkForItemChange(); + }); } void NameValueItemsWidget::setEnvironmentItems(const EnvironmentItems &items) { - EnvironmentItems sortedItems = items; - EnvironmentItem::sort(&sortedItems); - const QStringList list = EnvironmentItem::toStringList(sortedItems); - m_editor->document()->setPlainText(list.join(QLatin1Char('\n'))); + m_originalItems = items; + m_editor->document()->setPlainText(EnvironmentItem::toStringList(items) + .join(QLatin1Char('\n'))); } EnvironmentItems NameValueItemsWidget::environmentItems() const { const QStringList list = m_editor->document()->toPlainText().split(QLatin1String("\n")); - EnvironmentItems items = EnvironmentItem::fromStringList(list); - return cleanUp(items); + return Internal::cleanUp(EnvironmentItem::fromStringList(list)); } void NameValueItemsWidget::setPlaceholderText(const QString &text) @@ -87,13 +105,61 @@ void NameValueItemsWidget::setPlaceholderText(const QString &text) m_editor->setPlaceholderText(text); } -} // namespace Internal +bool NameValueItemsWidget::editVariable(const QString &name, Selection selection) +{ + QTextDocument * const doc = m_editor->document(); + for (QTextBlock b = doc->lastBlock(); b.isValid(); b = b.previous()) { + const QString &line = b.text(); + qsizetype offset = 0; + const auto skipWhiteSpace = [&] { + for (; offset < line.length(); ++offset) { + if (!line.at(offset).isSpace()) + return; + } + }; + skipWhiteSpace(); + if (line.mid(offset, name.size()) != name) + continue; + offset += name.size(); + + const auto updateCursor = [&](int anchor, int pos) { + QTextCursor newCursor(doc); + newCursor.setPosition(anchor); + newCursor.setPosition(pos, QTextCursor::KeepAnchor); + m_editor->setTextCursor(newCursor); + }; + + if (selection == Selection::Name) { + m_editor->setFocus(); + updateCursor(b.position() + offset, b.position()); + return true; + } + + skipWhiteSpace(); + if (offset < line.length()) { + QChar nextChar = line.at(offset); + if (nextChar.isLetterOrNumber()) + continue; + if (nextChar == '=') { + if (++offset < line.length() && line.at(offset) == '+') + ++offset; + } else if (nextChar == '+') { + if (++offset < line.length() && line.at(offset) == '=') + ++offset; + } + } + m_editor->setFocus(); + updateCursor(b.position() + b.length() - 1, b.position() + offset); + return true; + } + return false; +} NameValuesDialog::NameValuesDialog(const QString &windowTitle, QWidget *parent) : QDialog(parent) { resize(640, 480); - m_editor = new Internal::NameValueItemsWidget(this); + m_editor = new NameValueItemsWidget(this); auto box = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal, this); @@ -143,3 +209,5 @@ std::optional NameValuesDialog::getNameValueItems(QWidget *paren } } // namespace Utils + +#include diff --git a/src/libs/utils/namevaluesdialog.h b/src/libs/utils/namevaluesdialog.h index 50778378f15..af847bb5a1d 100644 --- a/src/libs/utils/namevaluesdialog.h +++ b/src/libs/utils/namevaluesdialog.h @@ -3,19 +3,39 @@ #pragma once -#include "utils_global.h" - #include "namevalueitem.h" +#include "utils_global.h" #include #include -#include #include namespace Utils { -namespace Internal { class NameValueItemsWidget; } +namespace Internal { class TextEditHelper; } + +class QTCREATOR_UTILS_EXPORT NameValueItemsWidget : public QWidget +{ + Q_OBJECT +public: + explicit NameValueItemsWidget(QWidget *parent = nullptr); + + void setEnvironmentItems(const EnvironmentItems &items); + EnvironmentItems environmentItems() const; + + void setPlaceholderText(const QString &text); + + enum class Selection { Name, Value }; + bool editVariable(const QString &name, Selection selection); + +signals: + void userChangedItems(const EnvironmentItems &items); + +private: + Internal::TextEditHelper *m_editor; + NameValueItems m_originalItems; +}; class QTCREATOR_UTILS_EXPORT NameValuesDialog : public QDialog { @@ -36,7 +56,7 @@ protected: QWidget *parent = {}); private: - Internal::NameValueItemsWidget *m_editor; + NameValueItemsWidget *m_editor; }; } // namespace Utils diff --git a/src/plugins/projectexplorer/environmentwidget.cpp b/src/plugins/projectexplorer/environmentwidget.cpp index 64e406ace22..9a63d789ece 100644 --- a/src/plugins/projectexplorer/environmentwidget.cpp +++ b/src/plugins/projectexplorer/environmentwidget.cpp @@ -125,38 +125,19 @@ private: PathTreeWidget m_view; }; -class EnvironmentDelegate : public QStyledItemDelegate -{ -public: - EnvironmentDelegate(Utils::EnvironmentModel *model, - QTreeView *view) - : QStyledItemDelegate(view), m_model(model), m_view(view) - {} - - QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const override - { - QWidget *w = QStyledItemDelegate::createEditor(parent, option, index); - if (index.column() != 0) - return w; - - if (auto edit = qobject_cast(w)) - edit->setValidator(new Utils::NameValueValidator( - edit, m_model, m_view, index, Tr::tr("Variable already exists."))); - return w; - } -private: - Utils::EnvironmentModel *m_model; - QTreeView *m_view; -}; - - //// // EnvironmentWidget::EnvironmentWidget //// -class EnvironmentWidgetPrivate +class EnvironmentWidget::Private { public: + Private(EnvironmentWidget *q) : q(q) {} + + void handleEditRequest(NameValueItemsWidget::Selection selection); + + EnvironmentWidget * const q; + Utils::NameValueItemsWidget m_editor; Utils::EnvironmentModel *m_model; EnvironmentWidget::Type m_type = EnvironmentWidget::TypeLocal; QString m_baseEnvironmentText; @@ -168,14 +149,13 @@ public: QPushButton *m_resetButton; QPushButton *m_unsetButton; QPushButton *m_toggleButton; - QPushButton *m_batchEditButton; QPushButton *m_appendPathButton = nullptr; QPushButton *m_prependPathButton = nullptr; QPushButton *m_terminalButton; }; EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additionalDetailsWidget) - : QWidget(parent), d(std::make_unique()) + : QWidget(parent), d(std::make_unique(this)) { d->m_model = new Utils::EnvironmentModel(); d->m_type = type; @@ -183,14 +163,27 @@ EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additi this, &EnvironmentWidget::userChangesChanged); connect(d->m_model, &QAbstractItemModel::modelReset, this, &EnvironmentWidget::invalidateCurrentIndex); - connect(d->m_model, &Utils::EnvironmentModel::focusIndex, this, &EnvironmentWidget::focusIndex); + // The text edit represents the raw, unexpanded user changes. + // There are two possible data flows: + // a) text edit -> model -> view for when the user types in the text edit + // b) top level widget + // -> text edit + // -> model -> view + // for when convenience functionality is used via the buttons. + // So the model is always updated from the text edit or text edit and model + // are updated in sync from this widget. + connect(&d->m_editor, &NameValueItemsWidget::userChangedItems, + d->m_model, &EnvironmentModel::setUserChanges); + auto vbox = new QVBoxLayout(this); vbox->setContentsMargins(0, 0, 0, 0); d->m_detailsContainer = new Utils::DetailsWidget(this); + connect(d->m_detailsContainer, &DetailsWidget::expanded, + this, &EnvironmentWidget::updateSummaryText); auto details = new QWidget(d->m_detailsContainer); d->m_detailsContainer->setWidget(details); @@ -209,7 +202,6 @@ EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additi tree, [tree](const QModelIndex &idx) { tree->edit(idx); }); d->m_environmentView = tree; d->m_environmentView->setModel(d->m_model); - d->m_environmentView->setItemDelegate(new EnvironmentDelegate(d->m_model, d->m_environmentView)); d->m_environmentView->setMinimumHeight(400); d->m_environmentView->setRootIsDecorated(false); const auto stretcher = new HeaderViewStretcher(d->m_environmentView->header(), 1); @@ -218,7 +210,7 @@ EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additi connect(d->m_model, &EnvironmentModel::userChangesChanged, stretcher, &HeaderViewStretcher::softStretch); d->m_environmentView->setSelectionMode(QAbstractItemView::SingleSelection); - d->m_environmentView->setSelectionBehavior(QAbstractItemView::SelectItems); + d->m_environmentView->setSelectionBehavior(QAbstractItemView::SelectRows); d->m_environmentView->setFrameShape(QFrame::NoFrame); QFrame *findWrapper = Core::ItemViewFind::createSearchableWrapper(d->m_environmentView, Core::ItemViewFind::LightColored); findWrapper->setFrameStyle(QFrame::StyledPanel); @@ -266,10 +258,6 @@ EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additi this, &EnvironmentWidget::prependPathButtonClicked); } - d->m_batchEditButton = new QPushButton(this); - d->m_batchEditButton->setText(Tr::tr("&Batch Edit...")); - buttonLayout->addWidget(d->m_batchEditButton); - d->m_terminalButton = new QPushButton(this); d->m_terminalButton->setText(Tr::tr("Open &Terminal")); d->m_terminalButton->setToolTip(Tr::tr("Open a terminal with this environment set up.")); @@ -278,6 +266,7 @@ EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additi buttonLayout->addStretch(); horizontalLayout->addLayout(buttonLayout); + horizontalLayout->addWidget(&d->m_editor); vbox2->addLayout(horizontalLayout); vbox->addWidget(d->m_detailsContainer); @@ -293,8 +282,6 @@ EnvironmentWidget::EnvironmentWidget(QWidget *parent, Type type, QWidget *additi this, &EnvironmentWidget::removeEnvironmentButtonClicked); connect(d->m_unsetButton, &QAbstractButton::clicked, this, &EnvironmentWidget::unsetEnvironmentButtonClicked); - connect(d->m_batchEditButton, &QAbstractButton::clicked, - this, &EnvironmentWidget::batchEditEnvironmentButtonClicked); connect(d->m_environmentView->selectionModel(), &QItemSelectionModel::currentChanged, this, &EnvironmentWidget::environmentCurrentIndexChanged); connect(d->m_terminalButton, &QAbstractButton::clicked, @@ -354,7 +341,7 @@ Utils::EnvironmentItems EnvironmentWidget::userChanges() const void EnvironmentWidget::setUserChanges(const Utils::EnvironmentItems &list) { d->m_model->setUserChanges(list); - updateSummaryText(); + d->m_editor.setEnvironmentItems(list); } void EnvironmentWidget::setOpenTerminalFunc(const EnvironmentWidget::OpenTerminalFunc &func) @@ -370,6 +357,12 @@ void EnvironmentWidget::expand() void EnvironmentWidget::updateSummaryText() { + // The summary is redundant with the text edit, so we hide it on expansion. + if (d->m_detailsContainer->state() == DetailsWidget::Expanded) { + d->m_detailsContainer->setSummaryText({}); + return; + } + Utils::EnvironmentItems list = d->m_model->userChanges(); Utils::EnvironmentItem::sort(&list); @@ -428,30 +421,52 @@ void EnvironmentWidget::updateButtons() void EnvironmentWidget::editEnvironmentButtonClicked() { - const QModelIndex current = d->m_environmentView->currentIndex(); - if (current.column() == 1 - && d->m_type == TypeLocal - && d->m_model->currentEntryIsPathList(current)) { - PathListDialog dlg(d->m_model->indexToVariable(current), - d->m_model->data(current).toString(), this); - if (dlg.exec() == QDialog::Accepted) - d->m_model->setData(current, dlg.paths()); - } else { - d->m_environmentView->edit(current); + d->handleEditRequest(NameValueItemsWidget::Selection::Value); +} + +void EnvironmentWidget::Private::handleEditRequest(NameValueItemsWidget::Selection selection) +{ + QModelIndex current = m_environmentView->currentIndex(); + if (current.column() == 0) + current = current.siblingAtColumn(1); + const QString varName = m_model->indexToVariable(current); + + // Known path lists are edited via a dedicated widget. + if (m_type == TypeLocal && m_model->currentEntryIsPathList(current)) { + PathListDialog dlg(varName, m_model->data(current).toString(), q); + if (dlg.exec() == QDialog::Accepted) { + m_model->setData(current, dlg.paths()); + m_editor.setEnvironmentItems(m_model->userChanges()); + } + return; } + + if (m_editor.editVariable(varName, selection)) + return; + + // The variable does not appear on an LHS in the text edit. This means it is not + // set or unset in any of the user changes. Therefore, we have to create a new + // change item for the user to edit. + EnvironmentItems items = m_model->userChanges(); + items.append({varName, "NEWVALUE"}); + q->setUserChanges(items); + const bool editable = m_editor.editVariable(varName, NameValueItemsWidget::Selection::Value); + QTC_CHECK(editable); } void EnvironmentWidget::addEnvironmentButtonClicked() { QModelIndex index = d->m_model->addVariable(); + d->m_editor.setEnvironmentItems(d->m_model->userChanges()); d->m_environmentView->setCurrentIndex(index); - d->m_environmentView->edit(index); + d->handleEditRequest(NameValueItemsWidget::Selection::Name); } void EnvironmentWidget::removeEnvironmentButtonClicked() { const QString &name = d->m_model->indexToVariable(d->m_environmentView->currentIndex()); d->m_model->resetVariable(name); + d->m_editor.setEnvironmentItems(d->m_model->userChanges()); } // unset in Merged Environment Mode means, unset if it comes from the base environment @@ -463,6 +478,7 @@ void EnvironmentWidget::unsetEnvironmentButtonClicked() d->m_model->resetVariable(name); else d->m_model->unsetVariable(name); + d->m_editor.setEnvironmentItems(d->m_model->userChanges()); } void EnvironmentWidget::amendPathList(Utils::NameValueItem::Operation op) @@ -473,7 +489,7 @@ void EnvironmentWidget::amendPathList(Utils::NameValueItem::Operation op) return; Utils::NameValueItems changes = d->m_model->userChanges(); changes.append({varName, dir.toUserOutput(), op}); - d->m_model->setUserChanges(changes); + setUserChanges(changes); } void EnvironmentWidget::appendPathButtonClicked() @@ -486,16 +502,6 @@ void EnvironmentWidget::prependPathButtonClicked() amendPathList(Utils::NameValueItem::Prepend); } -void EnvironmentWidget::batchEditEnvironmentButtonClicked() -{ - const Utils::EnvironmentItems changes = d->m_model->userChanges(); - - const auto newChanges = Utils::EnvironmentDialog::getEnvironmentItems(this, changes); - - if (newChanges) - d->m_model->setUserChanges(*newChanges); -} - void EnvironmentWidget::environmentCurrentIndexChanged(const QModelIndex ¤t) { if (current.isValid()) { diff --git a/src/plugins/projectexplorer/environmentwidget.h b/src/plugins/projectexplorer/environmentwidget.h index b130de3edb2..a00e45bcda7 100644 --- a/src/plugins/projectexplorer/environmentwidget.h +++ b/src/plugins/projectexplorer/environmentwidget.h @@ -17,8 +17,6 @@ QT_FORWARD_DECLARE_CLASS(QModelIndex) namespace ProjectExplorer { -class EnvironmentWidgetPrivate; - class PROJECTEXPLORER_EXPORT EnvironmentWidget : public QWidget { Q_OBJECT @@ -51,7 +49,6 @@ private: void unsetEnvironmentButtonClicked(); void appendPathButtonClicked(); void prependPathButtonClicked(); - void batchEditEnvironmentButtonClicked(); void environmentCurrentIndexChanged(const QModelIndex ¤t); void invalidateCurrentIndex(); void updateSummaryText(); @@ -62,7 +59,8 @@ private: using PathListModifier = std::function; void amendPathList(Utils::NameValueItem::Operation op); - const std::unique_ptr d; + class Private; + const std::unique_ptr d; }; } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/kitaspects.cpp b/src/plugins/projectexplorer/kitaspects.cpp index 281a59d25b1..f5f9a485aa3 100644 --- a/src/plugins/projectexplorer/kitaspects.cpp +++ b/src/plugins/projectexplorer/kitaspects.cpp @@ -1480,8 +1480,7 @@ private: if (HostOsInfo::isWindowsHost()) changes.removeAll(forceMSVCEnglishItem()); - return sorted(std::move(changes), [](const EnvironmentItem &lhs, const EnvironmentItem &rhs) - { return QString::localeAwareCompare(lhs.name, rhs.name) < 0; }); + return changes; } void initMSVCOutputSwitch(QVBoxLayout *layout)