Skip to content

Conversation

@taooceros
Copy link
Member

Also add a Cancellation Token field to ResultUpdatedEventArgs

@jjw24
Copy link
Member

jjw24 commented Jul 6, 2021

How are we planning to use IFeatures?

@jjw24
Copy link
Member

jjw24 commented Jul 6, 2021

How are we planning to use IFeatures?

All good, I just read the code lol

@jjw24
Copy link
Member

jjw24 commented Jul 6, 2021

Could we rename Feature.cs to Features.cs please

@jjw24
Copy link
Member

jjw24 commented Jul 6, 2021

Do we want to also move the other interfaces in to Features.cs or keep them separate?

@jjw24
Copy link
Member

jjw24 commented Jul 6, 2021

Could we rename Feature.cs to Features.cs please

Before you do this actually, will it break plugins ?

@taooceros
Copy link
Member Author

Could we rename Feature.cs to Features.cs please

Before you do this actually, will it break plugins ?

I test the Dictionary plugin, and it works. Not a lot plugin has implemented these interfaces.

@taooceros
Copy link
Member Author

Do we want to also move the other interfaces in to Features.cs or keep them separate?

I am not sure about this, but we should choose one throughoutly. Move all interface to one file or separate all of them.

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

just been checking this pr, ran into this issue:

image

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

also ran into this issue when downloaded dictionary plugin and trying to query it, doubt this may be related to this pr but can you see if you can replicate please

image

@taooceros
Copy link
Member Author

Will take a look now.

@taooceros
Copy link
Member Author

taooceros commented Jul 7, 2021

just been checking this pr, ran into this issue:

image

This issue has been reproduced before. The actual reason is unclear, probably because multithreading. It is debug only because on release there will be a default token.

@taooceros
Copy link
Member Author

also ran into this issue when downloaded dictionary plugin and trying to query it, doubt this may be related to this pr but can you see if you can replicate please

image

I will redownload dictionary plugin and try.

@taooceros
Copy link
Member Author

It is reproducible with current dev branch, but not reproducible with the pre-release build. Interesting

@taooceros
Copy link
Member Author

It is reproducible with current dev branch, but not reproducible with the pre-release build. I think the reason is that the release build already maintain a Sqlite.Interop.dll in the runtime, while when we load the plugin, we didn't load the runtime.

@taooceros
Copy link
Member Author

@jjw24 It's debug only issue, so won't stop us from checking this branch.

By the way, shall we add publishprofile to dotnet template?

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

publish profile for dontet template? dont think so, plugins releases zip file, what would be the need for a publish profile?

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

i renamed Features.cs. Checked that the only external plugins that uses interfaces from there are WindowWalker and Dictionary.

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

@jjw24 It's debug only issue, so won't stop us from checking this branch.

By the way, shall we add publishprofile to dotnet template?

oh you mean the github workflow file? then yes

@taooceros
Copy link
Member Author

@jjw24 It's debug only issue, so won't stop us from checking this branch.
By the way, shall we add publishprofile to dotnet template?

oh you mean the github workflow file? then yes

No, I mean auto create a publishprofile targeting the Net 5 and with runtime specification win-x64. Or else some native files like the Sqlite.Interop.dll will still reproduce the issue.

@taooceros
Copy link
Member Author

Though I think we can include a powershell script for creating release, and add the publish argument there.

@taooceros
Copy link
Member Author

i renamed Features.cs. Checked that the only external plugins that uses interfaces from there are WindowWalker and Dictionary.

And the everything plugin.

@taooceros taooceros requested a review from jjw24 July 7, 2021 10:42
@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

i renamed Features.cs. Checked that the only external plugins that uses interfaces from there are WindowWalker and Dictionary.

And the everything plugin.

yep everything plugin tested as well, is good.

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

Though I think we can include a powershell script for creating release, and add the publish argument there.

maybe a powershell better? dont think many will use publishprofile

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

can you see if you can replicate this:

in my ide vs 2019 in debug mode, when i turn on enable suggestions from google, no results are showing anymore. using the tray icon and going into settings to switch it off then restart, results come back again

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

can you see if you can replicate this:

in my ide vs 2019 in debug mode, when i turn on enable suggestions from google, no results are showing anymore. using the tray icon and going into settings to switch it off then restart, results come back again

possibly related to adding the cancellation token check in mainviewmodel

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

i dont think we should move interfaces into one file, hard to find them. Maybe just move them into the Interfaces folder and keep them seperate. If we keep in one file when we add more interfaces, it's going to be a big file in the future i think

@taooceros
Copy link
Member Author

can you see if you can replicate this:
in my ide vs 2019 in debug mode, when i turn on enable suggestions from google, no results are showing anymore. using the tray icon and going into settings to switch it off then restart, results come back again

possibly related to adding the cancellation token check in mainviewmodel

will check soon or tomorrow

@taooceros
Copy link
Member Author

I found out why you can reproduce the cancellation bug. Setting one of the web search with global wildcard with search source suggestion will reproduce the issue consistently.

@jjw24
Copy link
Member

jjw24 commented Jul 7, 2021

let's keep the interfaces in the Interfaces folder :)

@jjw24 jjw24 added enhancement New feature or request review in progress Indicates that a review is in progress for this PR and removed review in progress Indicates that a review is in progress for this PR labels Jul 7, 2021
@jjw24 jjw24 merged commit d623e3d into Flow-Launcher:dev Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants