New API Instance (ReQuery)

[AminSallah]

Changes:

• Added a new instance ReQuery in PublicAPIInstance.cs.
• Introduced a new instance ReQuery in IPublicAPI.cs.
• Changed the protection level of ReQuery to public.

Benefits:

This update allows plugin developers to reload queries to refresh data, addressing the following use cases:

• For counters and Pomodoro timers.
• Display cached results and, when new cache data is received from an online API, use ReQuery to update the query.
• Simultaneously refresh data.

Testing the ReQuery Instance

I have tested the ReQuery instance in one of my plugins to requery every second.

Task.Run(() =>
{
    Thread.Sleep(500);
    _context.API.ReQuery();
    Thread.Sleep(500);
});

Result:

Why not use ChangeQuery(Query.RawQuery, true):
The use of this method failed for me; it is too sticky. I cannot remove the action keyword or continue typing in the query. When triggering the flow, it changes the query on its own.

Task.Run(() =>
{
    Thread.Sleep(500);
    // _context.API.ReQuery();
    _context.API.ChangeQuery(query.RawQuery, true);
    Thread.Sleep(500);
});

Result:

[jjw24]

👍
Really well described PR, thank you for making the change requirement clear and easy to understand.

[jjw24]

Let's hold off merging till 1.18.0 release is scheduled.

[taooceros]

theoredically this can be achieved by ChangeQueryText(..., requery=true). Do we want to mark the second parameter as obsolete with this new api? Oh I see the description. It's a nice little api change!

[Garulf]

🤩

[AminSallah]

Hey there, I'd like to introduce an idea I've tested. However, I can't judge whether it enhances the user experience. I just want to share it.

Problem: By default, ReQuery sets the selected result to the first index all the time.

Task.Run(async () =>
{
    await this._notionDataParser.CallApiForSearch();
    Context.API.ReQuery();
});

Result
In this example, in a different thread, it waits for the API to update the local cache and then reloads the query using the default way.

Fix: I introduced a new boolean, reselect, in MainViewModel.cs and overridden the ReQuery method to ReQuery(bool reselect).

Task.Run(async () =>
{
    await this._notionDataParser.CallApiForSearch();
    Context.API.ReQuery(reselect: false);
});

Result
Same as before but keeps the selected result.

I cannot determine whether adding this option is better or not. It might not be more beneficial for large queries, like fetching 200 results, where users might not be noticed that the query has been reloaded. Alternatively, adding this option and allowing plugin developers to choose it could be an improvement. Feel free to share your thoughts on this feature.

Note: The method override keeps Ctrl + R unchanged.
Note: If the result after the reload is not present, Flow by default chooses the first one.

Link to branch let me know if you interested to push changes.

[taooceros]

interesting. I think this is easier to use compared to the UpdateResults event?

[AminSallah]

It's still using UpdateResults but passing a new parameter to it to prevent selecting the first index. I didn't attempt to come up with something new to reduce test time and potential issues that may arise.

[AminSallah]

Hey @jjw24 , Please see above comment.

[taooceros]

I don't like the idea of storing a variable for whether reselect. Probably it should be a parameter?

[AminSallah]

I am unsure if achieving smooth functionality is possible in this case, mainly due to the involvement of the RegisterViewUpdate method, which is invoking the UpdateResultView instance. This specific method is called by the Class structor and occurs subsequent to a query change and I encountered difficulty in passing a new parameter to it
https://github.com/AminSallah/Flow.Launcher/blob/99ac5f010d6136f44480b55274e1273b67e59b12/Flow.Launcher/ViewModel/MainViewModel.cs#L124

[taooceros]

I am unsure if achieving smooth functionality is possible in this case, mainly due to the involvement of the RegisterViewUpdate method, which is invoking the UpdateResultView instance. This specific method is called by the Class structor and occurs subsequent to a query change and I encountered difficulty in passing a new parameter to it https://github.com/AminSallah/Flow.Launcher/blob/99ac5f010d6136f44480b55274e1273b67e59b12/Flow.Launcher/ViewModel/MainViewModel.cs#L124

Oh probably add another property to ResultsForUpdate class?

[AminSallah]

How you'd suggest passing reselect param from ReQuery method() to RegisterViewUpdate() because calling RegisterViewUpdate() with new param causing a bug that flow not select first result by default any more after method is invoked, that's why i prefer storing a variable for whether reselect, it didn't raise any issues with me till now.

public void ReQuery(bool reselect)
{
    if (SelectedIsFromQueryResults())
    {
        QueryResults(isReQuery: true);
        RegisterViewUpdate(reselect);
    }
}

[taooceros]

How you'd suggest passing reselect param from ReQuery method() to RegisterViewUpdate() because calling RegisterViewUpdate() with new param causing a bug that flow not select first result by default any more after method is invoked, that's why i prefer storing a variable for whether reselect, it didn't raise any issues with me till now.

public void ReQuery(bool reselect)
{
    if (SelectedIsFromQueryResults())
    {
        QueryResults(isReQuery: true);
        RegisterViewUpdate(reselect);
    }
}

No that's not what I mean. I mean add a parameter to the UpdateResultView method (or add a property to the parameter with type IEnumerable<ResultsForUpdate>. This result would probably only makes sense when we only have one ResultsForUpdate (one plugin is responsible for one ResultsForUpdate).

It doesn't make sense to re-register everytime query.

[AminSallah]

Thanks, @taooceros, for your suggestion. I couldn't figure out a way to pass the parameter from ReQuery method to the UpdateResultView method due to my limited understanding of how the Flow Launcher backend works. So, if you have some time, please give it a try to make it.

[taooceros]

Sure let me show you what I mean.

[taooceros]

@AminSallah sorry for the delay. Take a look?

[AminSallah]

Yeah, now I understand, but do you prefer to add a parameter to the entire collection, needing only one to determine whether to select the first or not, rather than declaring it as a top class parameter? Also, I encountered a bug that I tried to fix, but couldn't figure out why it occurred. I double checked that the whole collection has the same ReselectFirstResult value.
When switching to any plugin, flow does not select a result at all in this scenario.

[taooceros]

I guess it's the problem we try to avoid by selecting the first result. Plugins result may appear at different time and some early result may get selected, and later move to some odd position.

[taooceros]

The parameter is not added to the entire collection of results, but only the result batch, which is slightly better. In fact, I think it is needed, because the results of different plugins can arrive at different times. They may not be processed at once?

I am doubting that the original implementation will only disable reSelect at the first results update but not in the later ones. Though I am not sure whether this is desirable or not.... This kind of reasoning can be the reason of the issue you mentioned.

[taooceros]

Class parameter introduce mutable state across various method, which might eventually make it very very hard to debug (what if we trigger a normal query right after some plugin trigger requery?). Flow already have a massive mutable states that makes it very hard to modify, so I would advocate that we should make things more pure.

Note: The local function (in the query result) still capture some state but that seems to be a little bit unavoidable unless we would like to pass the channel reader to the query function (which might be a good choice though).

[taooceros]

@AminSallah I found the issue...I made two parameter with the same when refactoring, which somehow create weird behavior.

[AminSallah]

The parameter is not added to the entire collection of results, but only the result batch, which is slightly better. In fact, I think it is needed, because the results of different plugins can arrive at different times. They may not be processed at once?

I noticed this just now. Currently, it's working very well. Much thanks to you @taooceros, for your assistance.

[taooceros]

You are very welcome.

[jjw24]

@AminSallah could you please resolve conflict.

I see this PR should be ready to go right, any additional from you @taooceros ?

[taooceros]

Yeah it's good for me

[github-actions]

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (23)

actionkeyword
actionkeywordassigned
cretain
eror
Fody
hotkeys
IPlugin
IPublic
keyevent
KListener
mainwindow
Mvvm
Pomodoro
refeshes
Reloadable
requery
requerying
Sallah
Segoe
spefic
viewupdate
vkcode
wpf

To accept these unrecognized words as correct, you could run the following commands

... in a clone of the git@github.com:AminSallah/Flow.Launcher.git repository
on the dev branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/prerelease/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/8462244429/attempts/1'

Errors (2)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors	Count
❌ check-file-path	1
ℹ️ non-alpha-in-dictionary	10

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it,
  try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.