-
-
Notifications
You must be signed in to change notification settings - Fork 406
Improve update logic & Fix update logic issue & Input for Query #3502
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
base: dev
Are you sure you want to change the base?
Changes from 1 commit
34c3cda
381334f
1381748
a2a4b5a
77b8749
07bcd5d
cf3fc6a
51714d5
ece9b96
8c48cbe
4c5b5fb
f599359
16dc921
8450e68
a47d8fe
bde7463
23a2f88
8670461
7c12956
a1df6a1
3bf6008
3786130
84e0193
2229db0
9136b19
81429d7
27ea2e4
1c0c9e7
5493e5c
a545a40
e23c8d8
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 | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -31,8 +31,9 @@ public partial class MainViewModel : BaseModel, ISavable, IDisposable | |||||||||
|
||||||||||
private static readonly string ClassName = nameof(MainViewModel); | ||||||||||
|
||||||||||
private bool _isQueryRunning; | ||||||||||
private Query _lastQuery; | ||||||||||
private Query _runningQuery; // Used for QueryResultAsync | ||||||||||
private Query _currentQuery; // Used for ResultsUpdated | ||||||||||
private string _queryTextBeforeLeaveResults; | ||||||||||
|
||||||||||
private readonly FlowLauncherJsonStorage<History> _historyItemsStorage; | ||||||||||
|
@@ -235,7 +236,7 @@ public void RegisterResultsUpdatedEvent() | |||||||||
var plugin = (IResultUpdated)pair.Plugin; | ||||||||||
plugin.ResultsUpdated += (s, e) => | ||||||||||
{ | ||||||||||
if (e.Query.RawQuery != QueryText || e.Token.IsCancellationRequested) | ||||||||||
if (_currentQuery == null || e.Query.RawQuery != _currentQuery.RawQuery || e.Token.IsCancellationRequested) | ||||||||||
{ | ||||||||||
return; | ||||||||||
} | ||||||||||
|
@@ -255,9 +256,12 @@ public void RegisterResultsUpdatedEvent() | |||||||||
|
||||||||||
PluginManager.UpdatePluginMetadata(resultsCopy, pair.Metadata, e.Query); | ||||||||||
|
||||||||||
if (token.IsCancellationRequested) return; | ||||||||||
if (_currentQuery == null || e.Query.RawQuery != _currentQuery.RawQuery || token.IsCancellationRequested) | ||||||||||
{ | ||||||||||
return; | ||||||||||
} | ||||||||||
|
||||||||||
if (!_resultsUpdateChannelWriter.TryWrite(new ResultsForUpdate(resultsCopy, pair.Metadata, e.Query, | ||||||||||
if (!_resultsUpdateChannelWriter.TryWrite(new ResultsForUpdate(resultsCopy, pair.Metadata, e.Query, | ||||||||||
token))) | ||||||||||
{ | ||||||||||
App.API.LogError(ClassName, "Unable to add item to Result Update Queue"); | ||||||||||
|
@@ -365,7 +369,7 @@ private void LoadContextMenu() | |||||||||
[RelayCommand] | ||||||||||
private void Backspace(object index) | ||||||||||
{ | ||||||||||
var query = QueryBuilder.Build(QueryText.Trim(), PluginManager.NonGlobalPlugins); | ||||||||||
var query = QueryBuilder.Build(QueryText, QueryText.Trim(), PluginManager.NonGlobalPlugins); | ||||||||||
|
||||||||||
// GetPreviousExistingDirectory does not require trailing '\', otherwise will return empty string | ||||||||||
var path = FilesFolders.GetPreviousExistingDirectory((_) => true, query.Search.TrimEnd('\\')); | ||||||||||
|
@@ -786,7 +790,7 @@ private ResultsViewModel SelectedResults | |||||||||
|
||||||||||
public Visibility ProgressBarVisibility { get; set; } | ||||||||||
public Visibility MainWindowVisibility { get; set; } | ||||||||||
|
||||||||||
// This is to be used for determining the visibility status of the main window instead of MainWindowVisibility | ||||||||||
// because it is more accurate and reliable representation than using Visibility as a condition check | ||||||||||
public bool MainWindowVisibilityStatus { get; set; } = true; | ||||||||||
|
@@ -1063,7 +1067,7 @@ private bool CanExternalPreviewSelectedResult(out string path) | |||||||||
path = QueryResultsPreviewed() ? Results.SelectedItem?.Result?.Preview.FilePath : string.Empty; | ||||||||||
return !string.IsNullOrEmpty(path); | ||||||||||
} | ||||||||||
|
||||||||||
private bool QueryResultsPreviewed() | ||||||||||
{ | ||||||||||
var previewed = PreviewSelectedItem == Results.SelectedItem; | ||||||||||
|
@@ -1196,6 +1200,7 @@ private void QueryHistory() | |||||||||
private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, bool reSelect = true) | ||||||||||
{ | ||||||||||
_updateSource?.Cancel(); | ||||||||||
_runningQuery = null; | ||||||||||
|
||||||||||
var query = await ConstructQueryAsync(QueryText, Settings.CustomShortcuts, Settings.BuiltinShortcuts); | ||||||||||
|
||||||||||
|
@@ -1215,89 +1220,103 @@ private async Task QueryResultsAsync(bool searchDelay, bool isReQuery = false, b | |||||||||
return; | ||||||||||
} | ||||||||||
|
||||||||||
_updateSource = new CancellationTokenSource(); | ||||||||||
try | ||||||||||
{ | ||||||||||
// Check if the query has changed because query can be changed so fast that | ||||||||||
// token of the query between two queries has not been created yet | ||||||||||
if (query.Input != QueryText) return; | ||||||||||
|
||||||||||
ProgressBarVisibility = Visibility.Hidden; | ||||||||||
_isQueryRunning = true; | ||||||||||
_updateSource = new CancellationTokenSource(); | ||||||||||
|
||||||||||
// Switch to ThreadPool thread | ||||||||||
await TaskScheduler.Default; | ||||||||||
ProgressBarVisibility = Visibility.Hidden; | ||||||||||
|
||||||||||
if (_updateSource.Token.IsCancellationRequested) return; | ||||||||||
_runningQuery = query; | ||||||||||
_currentQuery = query; | ||||||||||
|
||||||||||
// Update the query's IsReQuery property to true if this is a re-query | ||||||||||
query.IsReQuery = isReQuery; | ||||||||||
// Switch to ThreadPool thread | ||||||||||
await TaskScheduler.Default; | ||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
// handle the exclusiveness of plugin using action keyword | ||||||||||
RemoveOldQueryResults(query); | ||||||||||
if (_updateSource.Token.IsCancellationRequested) return; | ||||||||||
|
||||||||||
_lastQuery = query; | ||||||||||
// Update the query's IsReQuery property to true if this is a re-query | ||||||||||
query.IsReQuery = isReQuery; | ||||||||||
|
||||||||||
var plugins = PluginManager.ValidPluginsForQuery(query); | ||||||||||
// handle the exclusiveness of plugin using action keyword | ||||||||||
RemoveOldQueryResults(query); | ||||||||||
|
||||||||||
if (plugins.Count == 1) | ||||||||||
{ | ||||||||||
PluginIconPath = plugins.Single().Metadata.IcoPath; | ||||||||||
PluginIconSource = await App.API.LoadImageAsync(PluginIconPath); | ||||||||||
SearchIconVisibility = Visibility.Hidden; | ||||||||||
} | ||||||||||
else | ||||||||||
{ | ||||||||||
PluginIconPath = null; | ||||||||||
PluginIconSource = null; | ||||||||||
SearchIconVisibility = Visibility.Visible; | ||||||||||
} | ||||||||||
_lastQuery = query; | ||||||||||
|
||||||||||
// Do not wait for performance improvement | ||||||||||
/*if (string.IsNullOrEmpty(query.ActionKeyword)) | ||||||||||
{ | ||||||||||
// Wait 15 millisecond for query change in global query | ||||||||||
// if query changes, return so that it won't be calculated | ||||||||||
await Task.Delay(15, _updateSource.Token); | ||||||||||
if (_updateSource.Token.IsCancellationRequested) | ||||||||||
return; | ||||||||||
}*/ | ||||||||||
var plugins = PluginManager.ValidPluginsForQuery(query); | ||||||||||
|
||||||||||
_ = Task.Delay(200, _updateSource.Token).ContinueWith(_ => | ||||||||||
if (plugins.Count == 1) | ||||||||||
{ | ||||||||||
PluginIconPath = plugins.Single().Metadata.IcoPath; | ||||||||||
PluginIconSource = await App.API.LoadImageAsync(PluginIconPath); | ||||||||||
SearchIconVisibility = Visibility.Hidden; | ||||||||||
} | ||||||||||
else | ||||||||||
{ | ||||||||||
PluginIconPath = null; | ||||||||||
PluginIconSource = null; | ||||||||||
SearchIconVisibility = Visibility.Visible; | ||||||||||
} | ||||||||||
|
||||||||||
// Do not wait for performance improvement | ||||||||||
/*if (string.IsNullOrEmpty(query.ActionKeyword)) | ||||||||||
{ | ||||||||||
// Wait 15 millisecond for query change in global query | ||||||||||
// if query changes, return so that it won't be calculated | ||||||||||
await Task.Delay(15, _updateSource.Token); | ||||||||||
if (_updateSource.Token.IsCancellationRequested) | ||||||||||
return; | ||||||||||
}*/ | ||||||||||
|
||||||||||
_ = Task.Delay(200, _updateSource.Token).ContinueWith(_ => | ||||||||||
{ | ||||||||||
// start the progress bar if query takes more than 200 ms and this is the current running query and it didn't finish yet | ||||||||||
if (_isQueryRunning) | ||||||||||
if (_runningQuery != null && _runningQuery == query) | ||||||||||
{ | ||||||||||
ProgressBarVisibility = Visibility.Visible; | ||||||||||
} | ||||||||||
}, | ||||||||||
_updateSource.Token, | ||||||||||
TaskContinuationOptions.NotOnCanceled, | ||||||||||
TaskScheduler.Default); | ||||||||||
_updateSource.Token, | ||||||||||
TaskContinuationOptions.NotOnCanceled, | ||||||||||
TaskScheduler.Default); | ||||||||||
|
||||||||||
// plugins are ICollection, meaning LINQ will get the Count and preallocate Array | ||||||||||
// plugins are ICollection, meaning LINQ will get the Count and preallocate Array | ||||||||||
|
||||||||||
var tasks = plugins.Select(plugin => plugin.Metadata.Disabled switch | ||||||||||
{ | ||||||||||
false => QueryTaskAsync(plugin, _updateSource.Token), | ||||||||||
true => Task.CompletedTask | ||||||||||
}).ToArray(); | ||||||||||
var tasks = plugins.Select(plugin => plugin.Metadata.Disabled switch | ||||||||||
{ | ||||||||||
false => QueryTaskAsync(plugin, _updateSource.Token), | ||||||||||
true => Task.CompletedTask | ||||||||||
}).ToArray(); | ||||||||||
|
||||||||||
try | ||||||||||
{ | ||||||||||
// Check the code, WhenAll will translate all type of IEnumerable or Collection to Array, so make an array at first | ||||||||||
await Task.WhenAll(tasks); | ||||||||||
} | ||||||||||
catch (OperationCanceledException) | ||||||||||
{ | ||||||||||
// nothing to do here | ||||||||||
} | ||||||||||
try | ||||||||||
{ | ||||||||||
// Check the code, WhenAll will translate all type of IEnumerable or Collection to Array, so make an array at first | ||||||||||
await Task.WhenAll(tasks); | ||||||||||
} | ||||||||||
catch (OperationCanceledException) | ||||||||||
{ | ||||||||||
// nothing to do here | ||||||||||
} | ||||||||||
|
||||||||||
if (_updateSource.Token.IsCancellationRequested) return; | ||||||||||
if (_updateSource.Token.IsCancellationRequested) return; | ||||||||||
|
||||||||||
// this should happen once after all queries are done so progress bar should continue | ||||||||||
// until the end of all querying | ||||||||||
_isQueryRunning = false; | ||||||||||
// this should happen once after all queries are done so progress bar should continue | ||||||||||
// until the end of all querying | ||||||||||
_runningQuery = null; | ||||||||||
|
||||||||||
if (!_updateSource.Token.IsCancellationRequested) | ||||||||||
if (!_updateSource.Token.IsCancellationRequested) | ||||||||||
{ | ||||||||||
// update to hidden if this is still the current query | ||||||||||
ProgressBarVisibility = Visibility.Hidden; | ||||||||||
} | ||||||||||
} | ||||||||||
finally | ||||||||||
{ | ||||||||||
// update to hidden if this is still the current query | ||||||||||
ProgressBarVisibility = Visibility.Hidden; | ||||||||||
// this make sures running query is null even if the query is canceled | ||||||||||
_runningQuery = null; | ||||||||||
} | ||||||||||
|
||||||||||
// Local function | ||||||||||
|
@@ -1374,7 +1393,7 @@ private async Task<Query> ConstructQueryAsync(string queryText, IEnumerable<Cust | |||||||||
// Applying builtin shortcuts | ||||||||||
await BuildQueryAsync(builtInShortcuts, queryBuilder, queryBuilderTmp); | ||||||||||
|
||||||||||
return QueryBuilder.Build(queryBuilder.ToString().Trim(), PluginManager.NonGlobalPlugins); | ||||||||||
return QueryBuilder.Build(QueryText, queryBuilder.ToString().Trim(), PluginManager.NonGlobalPlugins); | ||||||||||
} | ||||||||||
Comment on lines
+1626
to
1627
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. Fix race: pass the method parameter as Input, not the mutable property. Using QueryText (property) here can capture a later value than the one used to construct the query, defeating the stale-query guard. Apply: - return QueryBuilder.Build(QueryText, queryBuilder.ToString().Trim(), PluginManager.NonGlobalPlugins);
+ return QueryBuilder.Build(queryText, queryBuilder.ToString().Trim(), PluginManager.NonGlobalPlugins); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||
|
||||||||||
private async Task BuildQueryAsync(IEnumerable<BaseBuiltinShortcutModel> builtInShortcuts, | ||||||||||
|
@@ -1549,9 +1568,6 @@ public bool ShouldIgnoreHotkeys() | |||||||||
|
||||||||||
public void Show() | ||||||||||
{ | ||||||||||
// When application is exiting, we should not show the main window | ||||||||||
if (App.Exiting) return; | ||||||||||
|
||||||||||
// When application is exiting, the Application.Current will be null | ||||||||||
Application.Current?.Dispatcher.Invoke(() => | ||||||||||
{ | ||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.