diff --git a/src/ui/CommitList.cpp b/src/ui/CommitList.cpp index 1531b5e47..4b118da7f 100644 --- a/src/ui/CommitList.cpp +++ b/src/ui/CommitList.cpp @@ -95,13 +95,32 @@ class CommitModel : public QAbstractListModel { // Connect watcher to signal when the status diff finishes. connect(&mStatus, &QFutureWatcher::finished, [this] { mTimer.stop(); - resetWalker(); - emit statusFinished(!mRows.isEmpty() && !mRows.first().commit.isValid()); + dispatchResetWalker(true); + }); + + // Apply the result of an asynchronous walker reset on the GUI thread. + connect(&mReset, &QFutureWatcher::finished, [this] { + ResetResult result = mReset.result(); + bool emitStatusFinished = result.emitStatusFinished; + applyResetResult(std::move(result)); + if (emitStatusFinished) + emit statusFinished(!mRows.isEmpty() && + !mRows.first().commit.isValid()); }); resetSettings(); } + ~CommitModel() { + // Ensure that mStatus is stopped since it captures `this` and potentially + // might crash after the destructor is finished + cancelStatus(); + + // ..and the same applies to mReset too + if (mReset.isRunning()) + mReset.waitForFinished(); + } + git::Reference reference() const { return mRef; } git::Diff status() const { @@ -124,6 +143,7 @@ class CommitModel : public QAbstractListModel { mRepo.index().read(); // Check for uncommitted changes asynchronously. + emit loadingChanged(true); mProgress = 0; mTimer.start(50); mStatus.setFuture(QtConcurrent::run([this] { @@ -177,65 +197,11 @@ class CommitModel : public QAbstractListModel { } } - void resetWalker() { - beginResetModel(); - - // Reset state. - mParents.clear(); - mRows.clear(); - DebugRefresh(""); - - // Update status row. - bool head = (!mRef.isValid() || mRef.isHead()); - bool valid = (!mStatus.isFinished() || status().isValid()); - if (mShowCleanStatus && head && valid && mPathspec.isEmpty()) { - QVector row; - if (mGraphVisible && mRef.isValid() && mStatus.isFinished()) { - row.append({Segment(Bottom, kTaintedColor), Segment(Dot, QColor())}); - mParents.append(Parent(mRef.target(), nextColor(), true)); - } - DebugRefresh("mRows append invalid commit"); - mRows.append(Row(git::Commit(), row)); // Uncommitted changes - } - - // Begin walking commits. - if (mRef.isValid()) { - int sort = GIT_SORT_NONE; - if (mGraphVisible) { - sort |= GIT_SORT_TOPOLOGICAL; - if (mSortDate) - sort |= GIT_SORT_TIME; - } else if (!mSortDate) { - sort |= GIT_SORT_TOPOLOGICAL; - } - - mWalker = mRef.walker( - sort, mRefsFilter == CommitList::RefsFilter::SelectedRefIgnoreMerge); - if (mRef.isLocalBranch()) { - // Add the upstream branch. - if (git::Branch upstream = git::Branch(mRef).upstream()) - mWalker.push(upstream); - } - - if (mRef.isHead()) { - // Add merge head. - if (git::Reference mergeHead = mRepo.lookupRef("MERGE_HEAD")) - mWalker.push(mergeHead); - } - - if (mRefsFilter == CommitList::RefsFilter::AllRefs) { - foreach (const git::Reference ref, mRepo.refs()) { - if (!ref.isStash()) - mWalker.push(ref); - } - } - } - - if (canFetchMore(QModelIndex())) - fetchMore(QModelIndex()); - - endResetModel(); - } + // Rebuild the walker and the first page of rows. The expensive part + // (building the revwalk over all refs and computing the graph for the + // first page of commits) runs on a background thread; see + // dispatchResetWalker(). + void resetWalker() { dispatchResetWalker(false); } void resetSettings(bool walk = false) { git::Config config = mRepo.appConfig(); @@ -254,71 +220,20 @@ class CommitModel : public QAbstractListModel { } void fetchMore(const QModelIndex &parent) { - // Load commits. - int i = 0; - QList rows; - git::Commit commit = mWalker.next(mPathspec); - while (commit.isValid()) { - // Add root commits. - bool root = false; - if (indexOf(commit) < 0) { - root = true; - mParents.append(Parent(commit, nextColor())); - } - - // Calculate graph columns. - // Remember current row. - QList parents = mParents; - - // Replace commit with its parents. - QList replacements; - foreach (const git::Commit &parent, commit.parents()) { - // FIXME: Mark commits that point to existing parent? - if (indexOf(parent) < 0 && !contains(parent, rows)) - replacements.append(parent); - if (mRefsFilter == CommitList::RefsFilter::SelectedRefIgnoreMerge) { - break; - } - } - - // Set parents for next row. - int index = indexOf(commit); - if (index >= 0) { - Parent parent = mParents.takeAt(index); - if (!replacements.isEmpty()) { - git::Commit replacement = replacements.takeFirst(); - mParents.insert(index, Parent(replacement, parent.color)); - foreach (const git::Commit &replacement, replacements) - mParents.append(Parent(replacement, nextColor())); - } - } - - // Add graph row. - QVector row; - if (mGraphVisible && mPathspec.isEmpty()) - row = columns(commit, parents, root); - - rows.append(Row(commit, row)); - DebugRefresh("Append commit: " << commit.shortId()); - - // Bail out. - if (i++ >= 64) - break; - - commit = mWalker.next(mPathspec); - } + FetchResult fetched = fetchRows(mWalker, mParents, mRows, mPathspec, + mGraphVisible, mRefsFilter); // Update the model. - if (!rows.isEmpty()) { + if (!fetched.rows.isEmpty()) { int first = mRows.size(); - int last = first + rows.size() - 1; + int last = first + fetched.rows.size() - 1; beginInsertRows(QModelIndex(), first, last); - mRows.append(rows); + mRows.append(fetched.rows); endInsertRows(); } // Invalidate walker. - if (!commit.isValid()) + if (fetched.exhausted) mWalker = git::RevWalk(); } @@ -403,6 +318,7 @@ class CommitModel : public QAbstractListModel { signals: void statusFinished(bool visible); + void loadingChanged(bool loading); private: struct Parent { @@ -436,23 +352,57 @@ class CommitModel : public QAbstractListModel { QVector columns; }; - int indexOf(const git::Commit &commit) const { - int count = mParents.size(); + // Everything the background thread needs to rebuild the walker and the + // first page of rows. Captured by value at dispatch time so the + // computation can run on another thread without touching model state. + struct ResetContext { + git::Reference ref; + QString pathspec; + bool graphVisible; + bool sortDate; + CommitList::RefsFilter refsFilter; + bool showCleanStatus; + git::Repository repo; + git::Diff statusDiff; + bool statusCheckFinished; + + // Carried straight through to ResetResult; see its field for why. + bool emitStatusFinished; + }; + + struct ResetResult { + QList parents; + QList rows; + git::RevWalk walker; + + // Whether this particular reset was triggered by the status check + // finishing, and should therefore emit statusFinished() once applied. + bool emitStatusFinished = false; + }; + + struct FetchResult { + QList rows; + bool exhausted = false; + }; + + int indexOf(const QList &parents, const git::Commit &commit) const { + int count = parents.size(); for (int i = 0; i < count; ++i) { - if (mParents.at(i).commit == commit) + if (parents.at(i).commit == commit) return i; } return -1; } - bool contains(const git::Commit &commit, const QList &rows) const { - foreach (const Row &row, mRows) { + bool contains(const git::Commit &commit, const QList &existingRows, + const QList &newRows) const { + for (const Row &row : existingRows) { if (row.commit == commit) return true; } - foreach (const Row &row, rows) { + for (const Row &row : newRows) { if (row.commit == commit) return true; } @@ -461,9 +411,10 @@ class CommitModel : public QAbstractListModel { } // The commit and parents parameters represent the current row. - // The mParents member represents the next row after this one. + // The nextParents parameter represents the next row after this one. QVector columns(const git::Commit &commit, - const QList &parents, bool root) { + const QList &parents, + const QList &nextParents, bool root) const { int count = parents.size(); QVector columns(count); @@ -486,14 +437,14 @@ class CommitModel : public QAbstractListModel { // Add a path to each successor. foreach (const git::Commit &successor, successors) { // Find index of parent in next row. - int index = indexOf(successor); + int index = indexOf(nextParents, successor); if (index < 0) continue; // Handle multiple commits that share the same parent. bool single = (successors.size() == 1); const QColor &color = - single ? parent.taintedColor(commit) : mParents.at(index).color; + single ? parent.taintedColor(commit) : nextParents.at(index).color; if (index < i) { // out to the left @@ -528,10 +479,10 @@ class CommitModel : public QAbstractListModel { return columns; } - QColor nextColor() { + QColor nextColor(const QList &parents) const { // Get the first unused (or least used) color. QMap counts; - foreach (const Parent &parent, mParents) + for (const Parent &parent : parents) counts[parent.color.name()]++; int count = 0; @@ -549,12 +500,178 @@ class CommitModel : public QAbstractListModel { return QColor(); } + // Walk at most one page of commits, updating parents in place and + // returning the new rows. Operates purely on its arguments (no access to + // 'this' state) so it can run on a background thread as well as + // synchronously from fetchMore(). + FetchResult fetchRows(git::RevWalk &walker, QList &parents, + const QList &existingRows, const QString &pathspec, + bool graphVisible, + CommitList::RefsFilter refsFilter) const { + FetchResult result; + int i = 0; + git::Commit commit = walker.next(pathspec); + while (commit.isValid()) { + // Add root commits. + bool root = false; + if (indexOf(parents, commit) < 0) { + root = true; + parents.append(Parent(commit, nextColor(parents))); + } + + // Calculate graph columns. + // Remember current row. + QList rowParents = parents; + + // Replace commit with its parents. + QList replacements; + for (const git::Commit &parent : commit.parents()) { + // FIXME: Mark commits that point to existing parent? + if (indexOf(parents, parent) < 0 && + !contains(parent, existingRows, result.rows)) + replacements.append(parent); + if (refsFilter == CommitList::RefsFilter::SelectedRefIgnoreMerge) { + break; + } + } + + // Set parents for next row. + int index = indexOf(parents, commit); + if (index >= 0) { + Parent parent = parents.takeAt(index); + if (!replacements.isEmpty()) { + git::Commit replacement = replacements.takeFirst(); + parents.insert(index, Parent(replacement, parent.color)); + for (const git::Commit &replacement : replacements) + parents.append(Parent(replacement, nextColor(parents))); + } + } + + // Add graph row. + QVector row; + if (graphVisible && pathspec.isEmpty()) + row = columns(commit, rowParents, parents, root); + + result.rows.append(Row(commit, row)); + DebugRefresh("Append commit: " << commit.shortId()); + + // Bail out. + if (i++ >= 64) + break; + + commit = walker.next(pathspec); + } + + result.exhausted = !commit.isValid(); + return result; + } + + // Build the walker and the first page of rows. Safe to run off the GUI + // thread: it only touches the context passed in and returns a fresh + // result rather than mutating model state directly. + ResetResult computeReset(const ResetContext &ctx) const { + ResetResult result; + + // Update status row. + bool head = (!ctx.ref.isValid() || ctx.ref.isHead()); + bool valid = (!ctx.statusCheckFinished || ctx.statusDiff.isValid()); + if (ctx.showCleanStatus && head && valid && ctx.pathspec.isEmpty()) { + QVector row; + if (ctx.graphVisible && ctx.ref.isValid() && ctx.statusCheckFinished) { + row.append({Segment(Bottom, kTaintedColor), Segment(Dot, QColor())}); + result.parents.append( + Parent(ctx.ref.target(), nextColor(result.parents), true)); + } + result.rows.append(Row(git::Commit(), row)); // Uncommitted changes + } + + // Begin walking commits. + if (ctx.ref.isValid()) { + int sort = GIT_SORT_NONE; + if (ctx.graphVisible) { + sort |= GIT_SORT_TOPOLOGICAL; + if (ctx.sortDate) + sort |= GIT_SORT_TIME; + } else if (!ctx.sortDate) { + sort |= GIT_SORT_TOPOLOGICAL; + } + + result.walker = ctx.ref.walker( + sort, + ctx.refsFilter == CommitList::RefsFilter::SelectedRefIgnoreMerge); + if (ctx.ref.isLocalBranch()) { + // Add the upstream branch. + if (git::Branch upstream = git::Branch(ctx.ref).upstream()) + result.walker.push(upstream); + } + + if (ctx.ref.isHead()) { + // Add merge head. + if (git::Reference mergeHead = ctx.repo.lookupRef("MERGE_HEAD")) + result.walker.push(mergeHead); + } + + if (ctx.refsFilter == CommitList::RefsFilter::AllRefs) { + for (const git::Reference &ref : ctx.repo.refs()) { + if (!ref.isStash()) + result.walker.push(ref); + } + } + } + + if (result.walker.isValid()) { + FetchResult fetched = + fetchRows(result.walker, result.parents, result.rows, ctx.pathspec, + ctx.graphVisible, ctx.refsFilter); + result.rows.append(fetched.rows); + if (fetched.exhausted) + result.walker = git::RevWalk(); + } + + result.emitStatusFinished = ctx.emitStatusFinished; + return result; + } + + // Kick off an asynchronous walker reset. The GUI thread keeps showing the + // previous rows (behind a loading indicator, see CommitList::setLoading) + // until the background computation finishes and applyResetResult() swaps + // the new data in. + void dispatchResetWalker(bool emitStatusFinishedAfter) { + ResetContext ctx{mRef, + mPathspec, + mGraphVisible, + mSortDate, + mRefsFilter, + mShowCleanStatus, + mRepo, + status(), + mStatus.isFinished(), + emitStatusFinishedAfter}; + + emit loadingChanged(true); + mReset.setFuture( + QtConcurrent::run([this, ctx] { return computeReset(ctx); })); + } + + // Apply a completed background reset on the GUI thread. + void applyResetResult(ResetResult &&result) { + beginResetModel(); + mParents = std::move(result.parents); + mRows = std::move(result.rows); + mWalker = std::move(result.walker); + DebugRefresh(""); + endResetModel(); + emit loadingChanged(false); + } + QTimer mTimer; int mProgress = 0; DiffCallbacks mStatusCallbacks; QFutureWatcher mStatus; + QFutureWatcher mReset; + QString mPathspec; git::Reference mRef; git::RevWalk mWalker; @@ -1192,6 +1309,13 @@ CommitList::CommitList(Index *index, QWidget *parent) mList = new ListModel(this); mModel = new CommitModel(repo, this); + connect(&mTimer, &QTimer::timeout, this, [this] { + ++mProgress; + if (mLoadingFadein < 1.0f) + mLoadingFadein += 0.1; + viewport()->update(); + }); + setMouseTracking(true); setUniformItemSizes(true); setAttribute(Qt::WA_MacShowFocusRect, false); @@ -1221,6 +1345,8 @@ CommitList::CommitList(Index *index, QWidget *parent) emit statusChanged(visible); }); + connect(model, &CommitModel::loadingChanged, this, &CommitList::setLoading); + git::RepositoryNotifier *notifier = repo.notifier(); connect(notifier, &git::RepositoryNotifier::referenceUpdated, [this](const git::Reference &ref, bool restoreSelection) { @@ -1365,6 +1491,10 @@ void CommitList::selectFirstCommit(bool spontaneous) { } else { emit diffSelected(git::Diff()); } + + // This is the automatic fallback selection, not a deliberate pick, so a + // later background refresh is free to move it instead of pinning it here. + mSelectionIsDefault = true; } void CommitList::selectCommitRelative(int offset) { @@ -1466,6 +1596,9 @@ void CommitList::setModel(QAbstractItemModel *model) { update(this->model()->index(row - 1, 0)); } + // Assume this selection is deliberate + mSelectionIsDefault = false; + notifySelectionChanged(); }); @@ -1756,8 +1889,42 @@ void CommitList::leaveEvent(QEvent *event) { QListView::leaveEvent(event); } +void CommitList::paintEvent(QPaintEvent *event) { + QListView::paintEvent(event); + + if (mLoading) { + QPainter painter(viewport()); + QRect indicator(QPoint(0, 0), ProgressIndicator::size()); + indicator.moveCenter(viewport()->rect().center()); + ProgressIndicator::paint(&painter, indicator, + palette().color(QPalette::WindowText), + mLoadingFadein, mProgress); + } +} + +void CommitList::setLoading(bool loading) { + if (loading == mLoading) + return; + + mLoading = loading; + if (loading) { + mLoadingFadein = 0; + mProgress = 0; + mTimer.start(50); + } else { + mTimer.stop(); + } + + viewport()->update(); + emit loadingChanged(loading); +} + void CommitList::storeSelection() { - mSelectedRange = selectedRange(); + // Don't pin the selection to a stale commit id across the reset: leave + // mSelectedRange empty so restoreSelection() defers to the fallback + // selection (selectFirstCommit(), triggered via statusFinished), which + // picks up whatever the new default is + mSelectedRange = mSelectionIsDefault ? QString() : selectedRange(); DebugRefresh("Selected Range: " << mSelectedRange); Debug(mSelectedRange); } @@ -1773,6 +1940,9 @@ void CommitList::restoreSelection() { } mSelectedRange = QString(); + + if (selectedIndexes().isEmpty()) + selectFirstCommit(); } void CommitList::updateModel() { @@ -1854,8 +2024,66 @@ void CommitList::notifySelectionChanged() { // Redraw all selected indexes. Separators may have changed. foreach (const QModelIndex &index, indexes) update(index); - git::Diff diff = selectedDiff(); - emit diffSelected(diff, mFile, mSpontaneous); + + dispatchSelectedDiff(mFile, mSpontaneous); +} + +void CommitList::dispatchSelectedDiff(const QString &file, bool spontaneous) { + // Any in-flight request is now stale. + int request = ++mDiffRequest; + + QModelIndexList indexes = sortedIndexes(); + if (indexes.isEmpty()) { + emit diffSelected(git::Diff(), file, spontaneous); + return; + } + + // The uncommitted-changes row's diff is already computed asynchronously + // elsewhere (CommitModel::status()); no need to compute it again. + if (indexes.size() == 1) { + git::Commit commit = indexes.first().data(CommitRole).value(); + if (!commit.isValid()) { + QVariant data = indexes.first().data(DiffRole); + git::Diff diff = data.isValid() ? data.value() : git::Diff(); + emit diffSelected(diff, file, spontaneous); + return; + } + } + + git::Commit first = indexes.first().data(CommitRole).value(); + if (!first.isValid()) { + emit diffSelected(git::Diff(), file, spontaneous); + return; + } + + git::Commit last = indexes.last().data(CommitRole).value(); + bool range = (indexes.size() > 1); + bool ignoreWhitespace = Settings::instance()->isWhitespaceIgnored(); + + // Let the diff/blame/file-list views clear themselves and show a loading + // indicator while the (potentially slow) diff is computed. + emit diffLoading(); + + // Compute the diff and run rename detection off the GUI thread; this can + // be slow for large commits/ranges. Discard the result if a newer + // selection has superseded this request by the time it finishes. + auto *watcher = new QFutureWatcher(this); + connect(watcher, &QFutureWatcher::finished, watcher, + [this, watcher, request, file, spontaneous] { + git::Diff diff = watcher->result(); + watcher->deleteLater(); + // TODO: It would be great to have some cancel pathway instead of + // doing this hack + if (request == mDiffRequest) + emit diffSelected(diff, file, spontaneous); + }); + + watcher->setFuture(QtConcurrent::run([first, last, range, ignoreWhitespace] { + git::Diff diff = range ? first.diff(last, -1, ignoreWhitespace) + : first.diff(git::Commit(), -1, ignoreWhitespace); + diff.findSimilar(); + return diff; + })); } bool CommitList::isDecoration(const QModelIndex &index, const QPoint &pos) { diff --git a/src/ui/CommitList.h b/src/ui/CommitList.h index 1454d7813..e65c8b3b6 100644 --- a/src/ui/CommitList.h +++ b/src/ui/CommitList.h @@ -12,6 +12,7 @@ #include "git/Reference.h" #include +#include class Index; @@ -63,22 +64,36 @@ class CommitList : public QListView { void setModel(QAbstractItemModel *model) override; + // Whether a status check and/or walker/row rebuild is currently in + // flight. See the loadingChanged() signal for a way to wait on this + // instead of polling it. + bool isLoading() const { return mLoading; } + signals: void statusChanged(bool dirty); void diffSelected(const git::Diff diff, const QString &file = QString(), bool spontaneous = false); + // Emitted just before a (potentially slow) diff is being computation. This + // can be used to clear GUI and enable loading indicators whilst waiting + void diffLoading(); + + // Emitted whenever isLoading() changes. + void loadingChanged(bool loading); + protected: void contextMenuEvent(QContextMenuEvent *event) override; void mouseMoveEvent(QMouseEvent *event) override; void mousePressEvent(QMouseEvent *event) override; void mouseReleaseEvent(QMouseEvent *event) override; void leaveEvent(QEvent *) override; + void paintEvent(QPaintEvent *event) override; private: void storeSelection(); void restoreSelection(); void updateModel(); + void setLoading(bool loading); QModelIndexList sortedIndexes() const; @@ -87,6 +102,7 @@ class CommitList : public QListView { const QString &file = QString(), bool spontaneous = false); void notifySelectionChanged(); + void dispatchSelectedDiff(const QString &file, bool spontaneous); bool isDecoration(const QModelIndex &index, const QPoint &pos); bool isStar(const QModelIndex &index, const QPoint &pos); @@ -105,6 +121,21 @@ class CommitList : public QListView { bool mRestoreSelection{true}; QString mSelectedRange; + + // Whether the current selection is just the automatic fallback rather + // than a deliberate user pick + bool mSelectionIsDefault{false}; + + // Whether the loading indicator should be shown + bool mLoading{false}; + float mLoadingFadein = 0; + int mProgress{0}; + QTimer mTimer; + + // Incremented on every selection-driven diff request. This is a hack used to + // discard diffs that arrive before the last one + // (Yes, we should have a proper cancel pathway here) + int mDiffRequest = 0; }; #endif diff --git a/src/ui/DetailView.cpp b/src/ui/DetailView.cpp index bb57f1201..57beeed88 100644 --- a/src/ui/DetailView.cpp +++ b/src/ui/DetailView.cpp @@ -618,6 +618,23 @@ void DetailView::setDiff(const git::Diff &diff, const QString &file, MenuBar::instance(this)->updateRepository(); } +void DetailView::setLoading() { + // Commit metadata (author, date, message, parents, refs, ...) comes from + // the selected commit(s), not the diff, so this can easily be shown + // immediatly. + RepoView *view = RepoView::parentView(this); + QList commits = view->commits(); + if (!commits.isEmpty()) { + mDetail->setCurrentIndex(CommitIndex); + mDetail->setVisible(true); + static_cast(mDetail->currentWidget())->setCommits(commits); + } + + // Incidate data loading while we wait for data to arrive + ContentWidget *cw = static_cast(mContent->currentWidget()); + cw->setLoading(); +} + void DetailView::cancelBackgroundTasks() { CommitDetail *cd = static_cast(mDetail->widget(CommitIndex)); cd->cancelBackgroundTasks(); diff --git a/src/ui/DetailView.h b/src/ui/DetailView.h index 3324e422f..5d52bb1f6 100644 --- a/src/ui/DetailView.h +++ b/src/ui/DetailView.h @@ -34,6 +34,13 @@ class ContentWidget : public QWidget { virtual void setDiff(const git::Diff &diff, const QString &file = QString(), const QString &pathspec = QString()) = 0; + /*! + * \brief Set whether or not to show a spinner. This is useful to indicate + * waiting for slow-content to arrive + * \param loading Indicator whether we wait for something to load + */ + virtual void setLoading() {} + virtual void cancelBackgroundTasks() {} virtual void find() {} @@ -72,6 +79,7 @@ class DetailView : public QWidget { void setCommitMessage(const QString &message); void setDiff(const git::Diff &diff, const QString &file = QString(), const QString &pathspec = QString()); + void setLoading(); void cancelBackgroundTasks(); diff --git a/src/ui/DiffTreeModel.cpp b/src/ui/DiffTreeModel.cpp index 44dffd0b0..703f01bc7 100644 --- a/src/ui/DiffTreeModel.cpp +++ b/src/ui/DiffTreeModel.cpp @@ -60,12 +60,14 @@ void DiffTreeModel::createDiffTree() { void DiffTreeModel::setDiff(const git::Diff &diff) { beginResetModel(); - if (diff) { - delete mRoot; - mDiff = diff; - mRoot = new Node(mRepo.workdir().path(), -1); + // Always rebuild the tree, even for an invalid diff, so callers that + // clear the diff (e.g. while a new one is loading) actually see an empty + // tree instead of the previous diff's stale rows. + delete mRoot; + mDiff = diff; + mRoot = new Node(mRepo.workdir().path(), -1); + if (diff) createDiffTree(); - } endResetModel(); } diff --git a/src/ui/DiffView/DiffView.cpp b/src/ui/DiffView/DiffView.cpp index 6f65a3534..cd4855826 100644 --- a/src/ui/DiffView/DiffView.cpp +++ b/src/ui/DiffView/DiffView.cpp @@ -16,7 +16,9 @@ #include "ui/DiffTreeModel.h" #include "ui/DoubleTreeWidget.h" #include "ui/HotkeyManager.h" +#include "ui/ProgressIndicator.h" #include "git/Tree.h" +#include #include #include #include @@ -100,10 +102,42 @@ DiffView::DiffView(const git::Repository &repo, QWidget *parent) shortcut = new QShortcut(this); moveHalfPageUpHotKey.use(shortcut); connect(shortcut, &QShortcut::activated, [this] { moveHalfPageUp(); }); + + connect(&mTimer, &QTimer::timeout, this, [this] { + ++mProgress; + if (mLoadingFadein < 1.0f) + mLoadingFadein += 0.1; + viewport()->update(); + }); } DiffView::~DiffView() {} +void DiffView::setLoading(bool loading) { + if (loading) { + mProgress = 0; + mLoadingFadein = 0; + mTimer.start(50); + } else { + mTimer.stop(); + } + + viewport()->update(); +} + +void DiffView::paintEvent(QPaintEvent *event) { + QScrollArea::paintEvent(event); + + if (!mDiff.isValid()) { + QPainter painter(viewport()); + QRect indicator(QPoint(0, 0), ProgressIndicator::size()); + indicator.moveCenter(viewport()->rect().center()); + ProgressIndicator::paint(&painter, indicator, + palette().color(QPalette::WindowText), + mLoadingFadein, mProgress); + } +} + QWidget *DiffView::file(int index) { fetchAll(index); return mFiles.at(index); @@ -429,8 +463,7 @@ void DiffView::fetchMore(int fetchWidgets) { } int count = indices.count(); - for (int i = mFiles.count(); i < count && addedWidgets < fetchWidgets; - ++i) { + for (int i = mFiles.count(); i < count && addedWidgets < fetchWidgets; ++i) { int pidx = indices[i].data(DiffTreeModel::PatchIndexRole).toInt(); git::Patch patch = mDiff.patch(pidx); diff --git a/src/ui/DiffView/DiffView.h b/src/ui/DiffView/DiffView.h index 262340123..12dcc0a74 100644 --- a/src/ui/DiffView/DiffView.h +++ b/src/ui/DiffView/DiffView.h @@ -21,6 +21,7 @@ #include "app/Theme.h" #include #include +#include class QCheckBox; class QVBoxLayout; @@ -99,6 +100,14 @@ class DiffView : public QScrollArea, public EditorProvider { * \param enable */ void enable(bool enable); + + /*! + * \brief Set whether or not to show a spinner. This is useful to indicate + * waiting for slow-content to arrive + * \param loading Indicator whether we wait for something to load + */ + void setLoading(bool loading); + void setModel(DiffTreeModel *model); void diffTreeModelDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, @@ -119,6 +128,7 @@ class DiffView : public QScrollArea, public EditorProvider { protected: void dropEvent(QDropEvent *event) override; void dragEnterEvent(QDragEnterEvent *event) override; + void paintEvent(QPaintEvent *event) override; private: bool canFetchMore(); @@ -142,6 +152,10 @@ class DiffView : public QScrollArea, public EditorProvider { DiffTreeModel *mDiffTreeModel{nullptr}; QWidget *mParent{nullptr}; QVBoxLayout *mFileWidgetLayout{nullptr}; + + float mLoadingFadein = 0; + int mProgress{0}; + QTimer mTimer; }; #endif diff --git a/src/ui/DoubleTreeWidget.cpp b/src/ui/DoubleTreeWidget.cpp index 93b40eec5..69c6c2247 100644 --- a/src/ui/DoubleTreeWidget.cpp +++ b/src/ui/DoubleTreeWidget.cpp @@ -403,6 +403,20 @@ QString DoubleTreeWidget::selectedFile() const { return ""; } +void DoubleTreeWidget::setLoading() { + // Clear the file list's rows, the diff view, and the blame editor, then + // let the file list and the diff view paint their own spinner over the + // now-empty content while we wait. + mDiffTreeModel->setDiff(git::Diff()); + + mEditor->clear(); + mDiffView->setDiff(git::Diff()); + + stagedFiles->setLoading(true); + unstagedFiles->setLoading(true); + mDiffView->setLoading(true); +} + /*! * \brief DoubleTreeWidget::setDiff * \param diff @@ -414,6 +428,11 @@ void DoubleTreeWidget::setDiff(const git::Diff &diff, const QString &file, Q_UNUSED(file) Q_UNUSED(pathspec) + // Diff is being set, so lets not indicate we're loading anything + stagedFiles->setLoading(false); + unstagedFiles->setLoading(false); + mDiffView->setLoading(false); + mSetDiffCounter++; DebugRefresh("time: " << QDateTime::currentDateTime() diff --git a/src/ui/DoubleTreeWidget.h b/src/ui/DoubleTreeWidget.h index 57cc246d9..b8be6fe00 100644 --- a/src/ui/DoubleTreeWidget.h +++ b/src/ui/DoubleTreeWidget.h @@ -41,6 +41,7 @@ class DoubleTreeWidget : public ContentWidget { void setDiff(const git::Diff &diff, const QString &file = QString(), const QString &pathspec = QString()) override; + void setLoading() override; void cancelBackgroundTasks() override; diff --git a/src/ui/ProgressIndicator.cpp b/src/ui/ProgressIndicator.cpp index 62b72e733..8ab7395ec 100644 --- a/src/ui/ProgressIndicator.cpp +++ b/src/ui/ProgressIndicator.cpp @@ -22,7 +22,7 @@ const int kSize = 26; QSize ProgressIndicator::size() { return QSize(kSize, kSize); } void ProgressIndicator::paint(QPainter *painter, const QRect &rect, - const QColor &c, int progress, + const QColor &c, float fadein, int progress, const QWidget *widget) { painter->save(); painter->setRenderHints(QPainter::Antialiasing); @@ -51,11 +51,11 @@ void ProgressIndicator::paint(QPainter *painter, const QRect &rect, const qreal in = 7; const qreal out = 12; - int alpha = 32; + int alpha = 32 * fadein; QColor color = c; for (int i = 0; i < 12; ++i) { color.setAlpha(alpha); - alpha += 16; + alpha += 16 * fadein; painter->setPen(QPen(color, 2.5, Qt::SolidLine, Qt::RoundCap)); diff --git a/src/ui/ProgressIndicator.h b/src/ui/ProgressIndicator.h index 1e82bda5f..944459286 100644 --- a/src/ui/ProgressIndicator.h +++ b/src/ui/ProgressIndicator.h @@ -17,8 +17,14 @@ class ProgressIndicator : public QWidget { public: static QSize size(); + static void paint(QPainter *painter, const QRect &rect, const QColor &c, + float fadein, int progress, + const QWidget *widget = nullptr); + static void paint(QPainter *painter, const QRect &rect, const QColor &color, - int progress, const QWidget *widget = nullptr); + int progress, const QWidget *widget = nullptr) { + paint(painter, rect, color, 1.0f, progress, widget); + } }; #endif diff --git a/src/ui/RepoView.cpp b/src/ui/RepoView.cpp index 547ecddff..e67f9e7a7 100644 --- a/src/ui/RepoView.cpp +++ b/src/ui/RepoView.cpp @@ -273,6 +273,8 @@ RepoView::RepoView(const git::Repository &repo, MainWindow *parent) connect(mRefs, &ReferenceWidget::referenceSelected, mCommits, &CommitList::selectReference); connect(mCommits, &CommitList::statusChanged, this, &RepoView::statusChanged); + connect(mCommits, &CommitList::loadingChanged, this, + &RepoView::loadingChanged); // Respond to pathspec change. connect(mPathspec, &PathspecWidget::pathspecChanged, this, @@ -306,6 +308,8 @@ RepoView::RepoView(const git::Repository &repo, MainWindow *parent) // Respond to commit list selection change. connect(mCommits, &CommitList::diffSelected, this, &RepoView::diffSelected, Qt::ConnectionType::DirectConnection); + connect(mCommits, &CommitList::diffLoading, mDetails, + &DetailView::setLoading); // Refresh the diff when a whole directory is added to the index. // FIXME: This is a workaround. @@ -505,6 +509,8 @@ RepoView::ViewMode RepoView::viewMode() const { return mDetails->viewMode(); } void RepoView::setViewMode(ViewMode mode) { mDetails->setViewMode(mode, true); } +bool RepoView::isLoading() const { return mCommits->isLoading(); } + bool RepoView::isWorkingDirectoryDirty() const { git::Diff status = mCommits->status(); if (!status.isValid()) diff --git a/src/ui/RepoView.h b/src/ui/RepoView.h index 6040ea863..25d9ff9e9 100644 --- a/src/ui/RepoView.h +++ b/src/ui/RepoView.h @@ -107,6 +107,10 @@ class RepoView : public QSplitter { // workdir bool isWorkingDirectoryDirty() const; + // Whether a status check and/or walker/row rebuild is currently in + // flight for the commit list. + bool isLoading() const; + // current reference git::Reference reference() const; void selectReference(const git::Reference &ref); @@ -360,6 +364,7 @@ private slots: signals: void statusChanged(bool dirty); + void loadingChanged(bool loading); protected: void showEvent(QShowEvent *event) override; diff --git a/src/ui/TreeView.cpp b/src/ui/TreeView.cpp index 770847e3e..5c0916e19 100644 --- a/src/ui/TreeView.cpp +++ b/src/ui/TreeView.cpp @@ -9,6 +9,7 @@ #include "TreeView.h" #include "ColumnView.h" +#include "ProgressIndicator.h" #include "ViewDelegate.h" #include "TreeModel.h" #include "Debug.h" @@ -48,6 +49,42 @@ TreeView::TreeView(QWidget *parent, const QString &name) mFileListDelegatePtr(std::make_unique(this, true)), mFileTreeDelegatePtr(std::make_unique(this)), mName(name) { setObjectName(name); + + connect(&mTimer, &QTimer::timeout, this, [this] { + ++mProgress; + if (mLoadingFadein < 1.0f) + mLoadingFadein += 0.1; + viewport()->update(); + }); +} + +void TreeView::setLoading(bool loading) { + if (loading == mLoading) + return; + + mLoading = loading; + if (loading) { + mProgress = 0; + mLoadingFadein = 0; + mTimer.start(50); + } else { + mTimer.stop(); + } + + viewport()->update(); +} + +void TreeView::paintEvent(QPaintEvent *event) { + QTreeView::paintEvent(event); + + if (mLoading) { + QPainter painter(viewport()); + QRect indicator(QPoint(0, 0), ProgressIndicator::size()); + indicator.moveCenter(viewport()->rect().center()); + ProgressIndicator::paint(&painter, indicator, + palette().color(QPalette::WindowText), + mLoadingFadein, mProgress); + } } void TreeView::updateView() { diff --git a/src/ui/TreeView.h b/src/ui/TreeView.h index ad489bf77..700190d1f 100644 --- a/src/ui/TreeView.h +++ b/src/ui/TreeView.h @@ -10,6 +10,7 @@ #ifndef TREEVIEW_H #define TREEVIEW_H +#include #include #include #include "ViewDelegate.h" @@ -43,6 +44,14 @@ class TreeView : public QTreeView { */ int countCollapsed(QModelIndex parent = QModelIndex(), bool recursive = true); void updateView(); + + /*! + * \brief Set whether or not to show a spinner. This is useful to indicate + * waiting for slow-content to arrive + * \param loading Indicator whether we wait for something to load + */ + void setLoading(bool loading); + public slots: /*! * \brief expandAll @@ -74,6 +83,9 @@ public slots: void filesSelected(const QModelIndexList &indexes); void collapseCountChanged(int count); +protected: + void paintEvent(QPaintEvent *event) override; + private: /*! * \brief setCollapseCount @@ -104,6 +116,11 @@ public slots: std::unique_ptr mFileListDelegatePtr; std::unique_ptr mFileTreeDelegatePtr; int mDelegateCol{false}; + + bool mLoading{false}; + float mLoadingFadein = 0; + int mProgress{0}; + QTimer mTimer; }; #endif // TREEVIEW_H diff --git a/test/Submodule.cpp b/test/Submodule.cpp index f66915aa5..7f0269568 100644 --- a/test/Submodule.cpp +++ b/test/Submodule.cpp @@ -61,14 +61,17 @@ void TestSubmodule::updateSubmoduleClone() { CloneDialog *d = new CloneDialog(CloneDialog::Kind::Clone); RepoView *view = nullptr; + MainWindow *window = nullptr; bool cloneFinished = false; - QObject::connect(d, &CloneDialog::accepted, [d, &view, &cloneFinished] { - cloneFinished = true; - if (MainWindow *window = MainWindow::open(d->path())) { - view = window->currentView(); - } - }); + QObject::connect(d, &CloneDialog::accepted, + [d, &window, &view, &cloneFinished] { + cloneFinished = true; + window = MainWindow::open(d->path()); + if (window) { + view = window->currentView(); + } + }); QTemporaryDir tempdir; QVERIFY(tempdir.isValid()); @@ -90,6 +93,14 @@ void TestSubmodule::updateSubmoduleClone() { QVERIFY(s.isValid()); QVERIFY(s.isInitialized()); } + + // Close the window (and its tabs) now, before tempdir's destructor below + // deletes the cloned repo out from under it -- otherwise it lingers as a + // dangling tab that later tests' sidebar refreshes can trip over. Window + // actually gone before this function (and tempdir) returns. + // deleted. + window->close(); + qWait(0); } void TestSubmodule::noUpdateSubmoduleClone() { @@ -102,14 +113,17 @@ void TestSubmodule::noUpdateSubmoduleClone() { CloneDialog *d = new CloneDialog(CloneDialog::Kind::Clone); RepoView *view = nullptr; + MainWindow *window = nullptr; bool cloneFinished = false; - QObject::connect(d, &CloneDialog::accepted, [d, &view, &cloneFinished] { - cloneFinished = true; - if (MainWindow *window = MainWindow::open(d->path())) { - view = window->currentView(); - } - }); + QObject::connect(d, &CloneDialog::accepted, + [d, &window, &view, &cloneFinished] { + cloneFinished = true; + window = MainWindow::open(d->path()); + if (window) { + view = window->currentView(); + } + }); QTemporaryDir tempdir; QVERIFY(tempdir.isValid()); @@ -131,6 +145,11 @@ void TestSubmodule::noUpdateSubmoduleClone() { QVERIFY(s.isValid()); QCOMPARE(s.isInitialized(), false); } + + // Close the window (and its tabs) now, before tempdir's destructor below + // updateSubmoduleClone(). + window->close(); + qWait(0); // let the WA_DeleteOnClose deferred deletion run now } void TestSubmodule::discardFile() { diff --git a/test/Test.cpp b/test/Test.cpp index 48de345ec..de65b419e 100644 --- a/test/Test.cpp +++ b/test/Test.cpp @@ -180,21 +180,37 @@ void Timeout::onTimeout() { } void refresh(RepoView *view, bool expectDirty) { - // Setup post refresh trigger. - bool finished = false; - auto connection = QObject::connect(view, &RepoView::statusChanged, - [&finished, expectDirty](bool dirty) { - QCOMPARE(dirty, expectDirty); - finished = true; + // Ensure that we're done loading before testing anything else + { + QEventLoop drainLoop; + bool loading = false; + auto connection = + QObject::connect(view, &RepoView::loadingChanged, &drainLoop, + [&drainLoop, &loading](bool nowLoading) { + loading = nowLoading; + if (!loading) + drainLoop.quit(); + }); + loading = view->isLoading(); + if (loading) + drainLoop.exec(); + QObject::disconnect(connection); + } + + // Wait for the refresh this call triggers to finish. + QEventLoop loop; + bool dirty = false; + auto connection = QObject::connect(view, &RepoView::statusChanged, &loop, + [&loop, &dirty](bool d) { + dirty = d; + loop.quit(); }); view->refresh(); - - // Wait for the refresh to finish. - while (!finished) - qWait(100); + loop.exec(); QObject::disconnect(connection); + QCOMPARE(dirty, expectDirty); // Select status index. if (expectDirty) diff --git a/test/TreeView.cpp b/test/TreeView.cpp index d85767227..694e58119 100644 --- a/test/TreeView.cpp +++ b/test/TreeView.cpp @@ -43,6 +43,7 @@ private slots: void restoreStagedFileAfterCommit(); void discardFiles(); void fileMergeCrash(); + void selectionSurvivesPush(); void dirtySubmoduleAndStagedSubmodule(); void conflictedAndStagedFile(); @@ -65,7 +66,7 @@ void TestTreeView::restoreStagedFileAfterCommit() { // Wait for refresh auto timeout = Timeout(10000, "Repository didn't refresh in time"); while (unstagedModel->rowCount() < 1) - qWait(300); + qWait(10); QCOMPARE(unstagedModel->rowCount(), 2); auto folder = unstagedModel->index(0, 0); @@ -157,7 +158,7 @@ void TestTreeView::discardFiles() { // Wait for refresh auto timeout = Timeout(10000, "Repository didn't refresh in time"); while (unstagedModel->rowCount() < 1) - qWait(300); + qWait(10); QCOMPARE(unstagedModel->rowCount(), 4); auto folder1 = unstagedModel->index(3, 0); @@ -231,8 +232,9 @@ void TestTreeView::fileMergeCrash() { QAbstractItemModel *stagedModel = stagedTree->model(); // Wait for refresh + auto timeout = Timeout(10000, "Repository didn't refresh in time"); while (stagedModel->rowCount() < 3) - qWait(300); + qWait(10); QAbstractItemModel *unstagedModel = unstagedTree->model(); QCOMPARE(unstagedModel->rowCount(), 1); @@ -278,6 +280,61 @@ void TestTreeView::fileMergeCrash() { // should not crash } +void TestTreeView::selectionSurvivesPush() { + // Pushing must not leave the diff/file views permanently blank: they're + // allowed to clear while the push's resulting ref update is processed, + // but must come back populated afterward. + QTemporaryDir remoteDir; + QVERIFY(remoteDir.isValid()); + git::Repository remoteRepo = git::Repository::init(remoteDir.path(), true); + QVERIFY(remoteRepo.isValid()); + + INIT_REPO("TreeViewCollapseCount.zip", true); + + git::Remote remote = repo.addRemote("origin", remoteDir.path()); + QVERIFY(remote.isValid()); + + // Set up tracking with an initial push, same as any already-established + // branch would have. This first push takes the setUpstream=true path, + // which (unlike a plain push) also goes through a HEAD-related ref + // update + repoView->push(remote, git::Reference(), QString(), true, false, false); + { + auto timeout = Timeout(10000, "Initial push didn't complete in time"); + while (!remoteRepo.lookupRef("refs/heads/master").isValid()) + qWait(10); + } + + // Create a second commit so the real push below has something to send + { + QFile file(repo.workdir().filePath("newfile.txt")); + QVERIFY(file.open(QFile::WriteOnly)); + file.write("content"); + file.close(); + repo.index().setStaged({"newfile.txt"}, true); + QVERIFY(repo.commit("second commit")); + } + + // Wait for the resulting selection/diff to settle before the real push. + { + auto timeout = Timeout(10000, "Selection didn't settle in time"); + while (!repoView->diff().isValid()) + qWait(10); + } + + // Pushing more commits on an already-tracked branch, i.e. setUpstream=false. + // That only updates the remote-tracking ref, not HEAD, so it must not leave + // the diff/file views permanently blank. + repoView->push(remote, git::Reference(), QString(), false, false, false); + + // The diff view must come back populated, not stay cleared. Without the + // fix this hits the Timeout below and aborts. + auto timeout = + Timeout(10000, "Diff view didn't get repopulated after the push"); + while (!repoView->diff().isValid()) + qWait(10); +} + void TestTreeView::dirtySubmoduleAndStagedSubmodule() { INIT_REPO("DirtySubmoduleUnstagedTree.zip", false); @@ -290,6 +347,14 @@ void TestTreeView::dirtySubmoduleAndStagedSubmodule() { { QAbstractItemModel *stagedModel = stagedTree->model(); + + { + // Wait for refresh + auto timeout = Timeout(10000, "Repository didn't refresh in time"); + while (stagedModel->rowCount() < 1) + qWait(10); + } + QCOMPARE(stagedModel->rowCount(), 1); QModelIndex index = stagedModel->index(0, 0); // submodules folder QVERIFY(index.isValid()); @@ -303,6 +368,13 @@ void TestTreeView::dirtySubmoduleAndStagedSubmodule() { { QAbstractItemModel *unstagedModel = unstagedTree->model(); + { + // Wait for refresh + auto timeout = Timeout(10000, "Repository didn't refresh in time"); + while (unstagedModel->rowCount() < 1) + qWait(300); + } + QCOMPARE(unstagedModel->rowCount(), 1); QModelIndex index = unstagedModel->index(0, 0); // submodules folder QVERIFY(index.isValid()); @@ -327,6 +399,14 @@ void TestTreeView::conflictedAndStagedFile() { { QAbstractItemModel *stagedModel = stagedTree->model(); + + { + // Wait for refresh + auto timeout = Timeout(10000, "Repository didn't refresh in time"); + while (stagedModel->rowCount() < 1) + qWait(10); + } + QCOMPARE(stagedModel->rowCount(), 1); QModelIndex index = stagedModel->index(0, 0); // "folder" folder QVERIFY(index.isValid()); @@ -340,6 +420,13 @@ void TestTreeView::conflictedAndStagedFile() { { QAbstractItemModel *unstagedModel = unstagedTree->model(); + { + // Wait for refresh + auto timeout = Timeout(10000, "Repository didn't refresh in time"); + while (unstagedModel->rowCount() < 1) + qWait(300); + } + QCOMPARE(unstagedModel->rowCount(), 1); QModelIndex index = unstagedModel->index(0, 0); // "folder" folder QVERIFY(index.isValid()); diff --git a/test/fileContextMenu.cpp b/test/fileContextMenu.cpp index b8600d5f5..4153f8508 100644 --- a/test/fileContextMenu.cpp +++ b/test/fileContextMenu.cpp @@ -62,12 +62,9 @@ void TestFileContextMenu::testDiscardFile() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"file.txt"}; FileContextMenu m(repoView, files, repo.index()); @@ -138,12 +135,9 @@ void TestFileContextMenu::testDiscardSubmodule() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"GittyupTestRepo"}; FileContextMenu m(repoView, files, repo.index()); @@ -219,12 +213,9 @@ void TestFileContextMenu::testDiscardFolder() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"folder1"}; FileContextMenu m(repoView, files, repo.index()); @@ -297,12 +288,9 @@ void TestFileContextMenu::testDiscardNothing() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files; // no files passed FileContextMenu m(repoView, files, repo.index()); @@ -366,12 +354,9 @@ void TestFileContextMenu::testIgnoreFile() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"file.txt"}; FileContextMenu m(repoView, files, repo.index(), repoView); @@ -421,12 +406,9 @@ void TestFileContextMenu::testIgnoreFileUntracked() { QVERIFY(file.write("Content of new file") > 0); } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"newFile.txt"}; FileContextMenu m(repoView, files, repo.index(), repoView); @@ -485,12 +467,9 @@ void TestFileContextMenu::testIgnoreFolder() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"folder1"}; FileContextMenu m(repoView, files, repo.index(), repoView); @@ -556,12 +535,9 @@ void TestFileContextMenu::testRemoveUntrackedFolder() { } } - // refresh repo - emit repo.notifier()->referenceUpdated(repo.head()); - QTest::qWait(10); // Wait until status is finished (own thread executed) - - // let the changes settle - QApplication::processEvents(); + // refresh repo and wait for the (asynchronous) status/selection update to + // actually finish, instead of hoping a fixed sleep was long enough. + Test::refresh(repoView); QStringList files = {"folder_new"}; FileContextMenu m(repoView, files, repo.index(), repoView); diff --git a/test/init_repo.cpp b/test/init_repo.cpp index e16445e8e..663e8746c 100644 --- a/test/init_repo.cpp +++ b/test/init_repo.cpp @@ -191,7 +191,15 @@ void TestInitRepo::editFile() { DiffView *diff = view->findChild(); QVERIFY(diff); - QToolButton *edit = diff->findChild("EditButton"); + // The file's diff (and the FileWidget/EditButton it builds) loads + // asynchronously, so it may not exist yet right after selecting the + // file. + QToolButton *edit = nullptr; + { + auto timeout = Timeout(10000, "Diff didn't finish loading in time"); + while (!(edit = diff->findChild("EditButton"))) + qWait(300); + } QVERIFY(edit); // Set up timer to dismiss the popup.