-
-
Notifications
You must be signed in to change notification settings - Fork 396
Fix history results clear logic & Make sure results cleared #3525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
3718796
54d50cf
5784f87
3484d08
021b451
6e473c8
c8989c3
3436ac9
08447da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1284,8 +1284,6 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
// Update the query's IsReQuery property to true if this is a re-query | ||
query.IsReQuery = isReQuery; | ||
|
||
|
||
|
||
ICollection<PluginPair> plugins = Array.Empty<PluginPair>(); | ||
if (currentIsHomeQuery) | ||
{ | ||
|
@@ -1342,6 +1340,7 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
|
||
// plugins are ICollection, meaning LINQ will get the Count and preallocate Array | ||
|
||
var resultsCleared = false; | ||
Task[] tasks; | ||
if (currentIsHomeQuery) | ||
{ | ||
|
@@ -1376,6 +1375,9 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |
// nothing to do here | ||
} | ||
|
||
// If results are not cleared, we need to clear the results | ||
if (!resultsCleared) Results.Clear(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really necessary? This could cause unexplained/unexpected flicker There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We must need it. This is used to make sure results are cleared when there are not any update tasks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think about one situation. Home page feature is off and you just change query text from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you say not any update task are you referring to one example where you toggle off home page in settings and results are not cleared? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me give you one demo video There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jjw24 It seems that I have issues when uploading video here. Please see more in Discord. |
||
if (currentCancellationToken.IsCancellationRequested) return; | ||
|
||
// this should happen once after all queries are done so progress bar should continue | ||
|
@@ -1445,6 +1447,11 @@ await PluginManager.QueryHomeForPluginAsync(plugin, query, token) : | |
{ | ||
App.API.LogError(ClassName, "Unable to add item to Result Update Queue"); | ||
} | ||
else | ||
{ | ||
// Only update the clear flag when we successfully write to the channel | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we actually get failure writes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should still take that into consideration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm we probably should take into consideration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that will cause issue as below There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted |
||
resultsCleared = true; | ||
} | ||
} | ||
|
||
void QueryHistoryTask(CancellationToken token) | ||
|
@@ -1458,11 +1465,21 @@ void QueryHistoryTask(CancellationToken token) | |
|
||
App.API.LogDebug(ClassName, $"Update results for history"); | ||
|
||
// Indicate if to clear existing results so to show only ones from plugins with action keywords | ||
var shouldClearExistingResults = ShouldClearExistingResults(query, currentIsHomeQuery); | ||
_lastQuery = query; | ||
_previousIsHomeQuery = currentIsHomeQuery; | ||
|
||
if (!_resultsUpdateChannelWriter.TryWrite(new ResultsForUpdate(results, _historyMetadata, query, | ||
token))) | ||
token, reSelect, shouldClearExistingResults))) | ||
{ | ||
App.API.LogError(ClassName, "Unable to add item to Result Update Queue"); | ||
} | ||
else | ||
{ | ||
// Only update the clear flag when we successfully write to the channel | ||
resultsCleared = true; | ||
} | ||
} | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.