Result comparison causing Query result's action to be outdated

[HorridModz]

I've been having a weird issue with my Add2Path plugin, which has been present ever since I started developing it. It seems that in my Query method (which returns the results for a query), query is getting outdated.

My plugin operates on one-word commands, like add and remove, which take the succeeding text in the query as the command's input:

path add foo - command add with input foo path remove bar - command remove with input bar

I've been noticing that the input part - foo and bar - doesn't always update. It seems the input stays the same, even when I change . This has been bugging me a lot, and it makes the plugin practically unusable. So I did some investigating. I replaced my Query method with a simple tester that tells me the "query" passed to it when I click the result:

public List<Result> Query(Query query)
    {
        return new List<Result>
        {
            new Result
            {
                Title = "test - view query",
                SubTitle = "view query",
                Action = c => { Context.API.ShowMsg("Query", query.Search); return false; }
            }
        };
	}

Sure enough, the strange behavior occurs here. It might be better to fire it up and see yourself, but I'll try to explain what I'm experiencing:

1. I open FlowLauncher and type my action keyword (path), so the query is path.
2. I click the test result and get a notification reporting that the query is blank (as it should be - after the action keyword path, the query is just blank)
3. I type the characters hello world, leading to the query being: path hello word.
4. I once again click the test result, and the notification once again reports that the query is blank (this is wrong; the query should be hello world)
5. Keep clicking, the buggy behavior still occurs and the reported query does not update
6. Now, delete the text path (leaving hello world intact), and retype it.
7. Click the test result. This time, it properly returns the query: hello world, rather than reporting that it is blank (this is right, though the query is the same as it was last time, in step 4)

So, why's it work the second time but not the first time? It seems that deleting and re-adding the action keyword (path) forces it to update. This means that simply changing the input - from blank to hello world does not trigger an update.

However, if I change the code to report the query directly passed into the Query method rather than the Action function in one of the results, it works properly:

public List<Result> Query(Query query)
    {
        return new List<Result>
        {
            new Result
            {
                Title = "Query",
                SubTitle = query.Search,
                Action = c => { return false; }
            }
        };
    }

This means that the Query method is getting passed the properly updated query parameter, as it should. However, the Action in the returned result has the old query parameter.

This perplexes me, as the Query method is creating a new Result every time it is called - and if the query parameter is correct, it should be passing it to the result's Action - right?

But no, the Action is old and only updates when the result visually updates. I'm not very experienced with C#, but there seems to be two possible things that could cause this:

1. The Result constructor is not working intuitively, perhaps using some kind of caching. Thus, when I initialize the Action field in the new Result I am constructing and returning, it is not really making a new one but giving me an old one - which still contains the old Action field with the old query value.
2. The Result constructor is indeed creating a new Result object when I initialize and return it, but the Action field is not being updated. Maybe because of some rule in C# having to do with delegates as fields? This seems unlikely as I can't see how it could happen, but it's a possibility as I don't understand how C# handles stuff under the hood.

Of course, I'm leading toward my first theory, which suggests something happening with FlowLauncher's handling of Results and their Actions rather than my plugin. But I'm completely perplexed here, and I wouldn't be surprised if I'm just doing something wrong and overlooking something silly.

If you'd like my full plugin's code, let me know (though I believe that the initial, published version of the plugin, which is in the official plugins directory, contains this bug itself).

[HorridModz]

@jjw24 @taooceros

[taooceros]

Yes there are some caching. You can invalidate the cache by modifying the title/subtitle/score. This is a subtle issue which we haven't solved.

[jjw24]

@taooceros can you expand more on your answer please, the behaviour of 'changing the input - from blank to hello world' should trigger a result update.

[JohnTheGr8]

@taooceros can you expand more on your answer please, the behaviour of 'changing the input - from blank to hello world' should trigger a result update.

@HorridModz is changing the query input, but the plugin results are visually the same as before, and Flow just keeps the old results that have been cached. This is mostly to avoid UI flickering iirc.

[jjw24]

Ok I see, this is actually to do with how we are comparing existing and updated result, it's comparing on title, subtitle and score. Therefore your updated result must have one of the three different to the existing result, that's why your second plugin example is working because subtitle is different.

[JohnTheGr8]

If I remember correctly, it's more than just Title, Subtitle and Score that affect this behavior. Pretty sure other fields that change the result visually (like TitleHighlightData and AutoCompleteText) also do.

In any case, this part of Flow needs to receive some attention. Should we move this issue to the main repo?

[HorridModz]

Woah, this seems much more complicated than what I originally thought. But why is this undocumented? And what even is the point of it? With absolutely no override?

[HorridModz]

Yes there are some caching. You can invalidate the cache by modifying the title/subtitle/score. This is a subtle issue which we haven't solved.

Interesting, is there a way to force invalidate the cache for results? Or some simple way to just reload Result. When I create a new Result, why would it cache? That seems so unintuitive, especially with no way to bypass caching.

[HorridModz]

Reference in new i

@JohnTheGr8 I put this here because I was unsure if it happened in other languages. From my perspective, this is a major issue if it can lead a developer like myself to have perfectly fine code that is bugging due to a weird, undocumented type of caching - so if it's embedded in the design, it has to at least be documented.

[HorridModz]

@HorridModz is changing the query input, but the plugin results are visually the same as before, and Flow just keeps the old results that have been cached. This is mostly to avoid UI flickering iirc.

Why should a visual cache be applied to something non-visual like the result Action? That sounds inherently flawed. Caching these 3 things but not caching the Action should make it so the UI won't flicker while fixing the issue.

[jjw24]

If I remember correctly, it's more than just Title, Subtitle and Score that affect this behavior. Pretty sure other fields that change the result visually (like TitleHighlightData and AutoCompleteText) also do.

In any case, this part of Flow needs to receive some attention. Should we move this issue to the main repo?

For reference this is the part of the code: https://github.com/Flow-Launcher/Flow.Launcher/blob/94288b3ebab9e0ff25c20483d3f2b8ddf21b34eb/Flow.Launcher.Plugin/Result.cs#L160

[jjw24]

I agree with documentation, and unfortunately we also struggle to keep it up like other projects.

Not sure about the caching part since I haven't worked on this part of the code so maybe the others could shed some light what it is, I just thought the result comparison code is the root cause of this issue.

Moving this to main.

[HorridModz]

You can invalidate the cache by modifying the title/subtitle/score.

@taooceros In the meantime, do you have a suggestion for how to do this? Perhaps I could change the score as a workaround, as I always have only one result? In my plugin, this makes it hell to debug and confusing to use - so I can't just pretend there's no issue.

[taooceros]

I don't think it is necessary to cache old result to avoid flickering. I will post a pr recently to remove the cache and see the result.

[taooceros]

You can invalidate the cache by modifying the title/subtitle/score.

@taooceros In the meantime, do you have a suggestion for how to do this? Perhaps I could change the score as a workaround, as I always have only one result? In my plugin, this makes it hell to debug and confusing to use - so I can't just pretend there's no issue.

Yes I think that's good workaround.

[dxp227]

I lost days on this bug 😂😭 I haven't been so baffled in a long time 😅

[HorridModz]

@dxp227

I lost days on this bug 😂😭 I haven't been so baffled in a long time 😅

I gave up on the bug and just published the plugin, thinking I was crazy - I didn't even realize something funny was going on until I went back to update it! And yes, I also spent days - glad you made it here. As I said, this NEEDS to be documented!

[HorridModz]

I don't think it is necessary to cache old result to avoid flickering. I will post a pr recently to remove the cache and see the result.

Any updates? @taooceros

[jjw24]

Fix has been merged, will be available in v1.20.0 release.