commit c0150c4548bbe7705cc3481faadc39722e0dc9ba
parent f7550bc512cd131c2ce08ad2bc1b087b54b21ae9
Author: cfillion <cfillion@users.noreply.github.com>
Date: Thu, 14 Sep 2017 20:47:06 -0400
refresh the browser only when necessary – fix fight for the about dialog's exclusivity
Diffstat:
7 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/src/browser.cpp b/src/browser.cpp
@@ -182,8 +182,8 @@ void Browser::onCommand(const int id, const int event)
else
break;
case IDCANCEL:
- if(m_loadState >= Loading)
- hide();
+ if(m_loadState == Loading)
+ hide(); // keep ourselves alive
else
close();
break;
@@ -387,12 +387,13 @@ void Browser::updateAbout()
void Browser::refresh(const bool stale)
{
- // Do nothing when called again when (or while) the index downloading
- // transaction finishes. populate() handles the next step of the loading process.
switch(m_loadState) {
- case Done:
+ case DeferredLoaded:
+ // We already processed this transaction immediately before.
m_loadState = Loaded;
+ return;
case Loading:
+ // Don't refresh again while currently refreshing.
return;
default:
break;
@@ -414,30 +415,30 @@ void Browser::refresh(const bool stale)
}
if(Transaction *tx = g_reapack->setupTransaction()) {
- const bool firstLoad = m_loadState == Init;
+ const bool isFirstLoad = m_loadState == Init;
m_loadState = Loading;
tx->fetchIndexes(remotes, stale);
tx->onFinish([=] {
- m_loadState = Done;
-
- if(firstLoad || isVisible())
+ if(isFirstLoad || isVisible()) {
populate(tx->getIndexes(remotes));
- else
+
+ // Ignore the next call to refreshBrowser() if we know we'll be
+ // requested to handle the very same transaction.
+ m_loadState = tx->receipt()->test(Receipt::RefreshBrowser) ?
+ DeferredLoaded : Loaded;
+ }
+ else {
+ // User manually asked to refresh the browser but closed the window
+ // before it could finished fetching the up to date indexes.
close();
+ }
});
tx->runTasks();
}
}
-void Browser::setFilter(const string &newFilter)
-{
- Win32::setWindowText(m_filterHandle, newFilter.c_str());
- updateFilter(); // don't wait for the timer, update now!
- SetFocus(m_filterHandle);
-}
-
void Browser::populate(const vector<IndexPtr> &indexes)
{
try {
@@ -476,6 +477,13 @@ void Browser::populate(const vector<IndexPtr> &indexes)
show();
}
+void Browser::setFilter(const string &newFilter)
+{
+ Win32::setWindowText(m_filterHandle, newFilter.c_str());
+ updateFilter(); // don't wait for the timer, update now!
+ SetFocus(m_filterHandle);
+}
+
void Browser::transferActions()
{
list<Entry *> oldActions;
diff --git a/src/browser.hpp b/src/browser.hpp
@@ -81,9 +81,9 @@ private:
enum LoadState {
Init,
- Loaded,
Loading,
- Done,
+ Loaded,
+ DeferredLoaded,
};
void onSelection();
diff --git a/src/reapack.cpp b/src/reapack.cpp
@@ -371,12 +371,15 @@ Transaction *ReaPack::setupTransaction()
void ReaPack::teardownTransaction()
{
+ const bool needRefresh = m_tx->receipt()->test(Receipt::RefreshBrowser);
+
delete m_tx;
m_tx = nullptr;
// Update the browser only after the transaction is deleted because
// it must be able to start a new one to load the indexes
- refreshBrowser();
+ if(needRefresh)
+ refreshBrowser();
}
void ReaPack::commitConfig()
diff --git a/src/receipt.hpp b/src/receipt.hpp
@@ -41,14 +41,19 @@ public:
RemovedFlag = 1<<2,
ExportedFlag = 1<<3,
ErrorFlag = 1<<4,
+ IndexChangedFlag = 1<<5,
+
+ PackageChanged = InstalledFlag | RemovedFlag,
+ RefreshBrowser = PackageChanged | IndexChangedFlag,
};
Receipt();
int flags() const { return m_flags; }
- bool test(int f) const { return (m_flags & f) != 0; }
+ bool test(Flag f) const { return (m_flags & f) != 0; }
bool empty() const;
+ void setIndexChanged() { m_flags |= IndexChangedFlag; }
void addInstall(const Version *, const Registry::Entry &);
void addRemoval(const Path &p);
void addRemovals(const std::set<Path> &);
diff --git a/src/report.cpp b/src/report.cpp
@@ -74,7 +74,7 @@ void Report::updateLabel()
label = "Operation failed. The following error(s) occured:";
else if(m_receipt->test(Receipt::ErrorFlag))
label = "The operation was partially completed (one or more errors occured):";
- else if(m_receipt->test(Receipt::InstalledFlag | Receipt::RemovedFlag))
+ else if(m_receipt->test(Receipt::PackageChanged))
label = "All done! Description of the changes:";
else
label = "Operation completed successfully!";
diff --git a/src/transaction.cpp b/src/transaction.cpp
@@ -146,7 +146,9 @@ void Transaction::fetchIndex(const Remote &remote, const bool stale,
dl->setName(remote.name());
dl->onFinish([=] {
- if(!dl->save())
+ if(dl->save())
+ m_receipt.setIndexChanged();
+ else
m_receipt.addError({FS::lastError(), dl->path().target().join()});
if(FS::exists(path))
diff --git a/test/receipt.cpp b/test/receipt.cpp
@@ -46,21 +46,25 @@ TEST_CASE("set receipt flags", M) {
Version ver("1.0", &pkg);
r.addInstall(&ver, {});
+ CHECK(r.test(Receipt::PackageChanged));
expected = Receipt::InstalledFlag;
}
SECTION("removal") {
r.addRemoval(Path("hello/world"));
+ CHECK(r.test(Receipt::PackageChanged));
expected = Receipt::RemovedFlag;
}
SECTION("export") {
r.addExport(Path("hello/world"));
+ CHECK_FALSE(r.test(Receipt::PackageChanged));
expected = Receipt::ExportedFlag;
}
SECTION("error") {
r.addError({"message", "context"});
+ CHECK_FALSE(r.test(Receipt::PackageChanged));
expected = Receipt::ErrorFlag;
}
@@ -87,6 +91,16 @@ TEST_CASE("set restart needed flag", M) {
REQUIRE(r.test(Receipt::RestartNeededFlag));
}
+TEST_CASE("set index changed flag", M) {
+ Receipt r;
+ REQUIRE_FALSE(r.test(Receipt::IndexChangedFlag));
+ REQUIRE_FALSE(r.test(Receipt::RefreshBrowser));
+
+ r.setIndexChanged();
+ REQUIRE(r.test(Receipt::IndexChangedFlag));
+ REQUIRE(r.test(Receipt::RefreshBrowser));
+}
+
TEST_CASE("format receipt page title", M) {
SECTION("singular") {
ReceiptPage page{vector<int>{1}, "Singular", "Plural"};
