Skip to content

Program plugin improvement #189

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Nov 11, 2020
Merged

Program plugin improvement #189

merged 14 commits into from
Nov 11, 2020

Conversation

@taooceros
Copy link
Member

@taooceros taooceros commented Oct 25, 2020 •
edited
Loading

This commit port some change from Wox authored by @bao-qian, which greatly improved the indexing time of win32 programs.
It also optimized a few code, and remove lock that seems insignificant.

  1. Optimize the query of Program plugin by using generic to concat win32 and uwp apps.
  2. Remove extra duplicate fuzzy search.
  3. Greatly improve index speed by changing the way of Directory enumeration.

@taooceros
Copy link
Member Author

taooceros commented Oct 25, 2020 •
edited
Loading

Question: Whether program plugin should query both description and name? Should description only be something that needs to displayed or people would like to query with it?
@jjw24 What do you think about it?

@taooceros taooceros force-pushed the ProgramPluginImprovement branch from 71cf2e2 to e7c4d83 Compare October 28, 2020 04:16
@taooceros
Copy link
Member Author

taooceros commented Nov 1, 2020 •
edited
Loading

The change should be ready for reviewing, and after using it for a while, I haven't found issues. @jjw24

@taooceros taooceros requested a review from jjw24 November 4, 2020 14:35
@jjw24
Copy link
Member

jjw24 commented Nov 10, 2020 •
edited
Loading

Question: Whether program plugin should query both description and name?

Would there be a significant benefit not querying description?

I suppose in theory querying the description helps where you dont know the name of the program.

Should description only be something that needs to displayed or people would like to query with it?

In terms of displaying the description, I am personally quite ok with it added to the name as it is. My own usage, i dont really query the description unless i absolutely dont know what the program is called.

I don't really know the impact, maybe put out a separate PR to test it out to get a better understanding?

@jjw24 jjw24 added review in progress Indicates that a review is in progress for this PR enhancement New feature or request labels Nov 10, 2020
jjw24
jjw24 reviewed Nov 10, 2020
View reviewed changes
Plugins/Flow.Launcher.Plugin.Program/Programs/UWP.cs
Comment on lines +271 to +280
var title = (Name, Description) switch
{
(var n, null) => n,
(var n, var d) when d.StartsWith(n) => d,
(var n, var d) when n.StartsWith(d) => n,
(var n, var d) when !string.IsNullOrEmpty(d) => $"{n}: {d}",
_ => Name
};

var matchResult = StringMatcher.FuzzySearch(query, title);
Copy link
Member

@jjw24 jjw24 Nov 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

@taooceros
Copy link
Member Author

taooceros commented Nov 10, 2020

Well there isn't a significant benefit for removing the behavior of search from description except the trivial less time of search, but generally I think few people will use descriptions to search the program, and @bao-qian has removed the entire description part in wox, so just propose that here. Because in my view, flow should be a launcher that help us launcher program fast, so the long description may not be useful for search fast.
It is just a proposal, and wait for other to provide ideas.

@jjw24
Copy link
Member

jjw24 commented Nov 10, 2020

Well there isn't a significant benefit for removing the behavior of search from description except the trivial less time of search, but generally I think few people will use descriptions to search the program, and @bao-qian has removed the entire description part in wox, so just propose that here. Because in my view, flow should be a launcher that help us launcher program fast, so the long description may not be useful for search fast.
It is just a proposal, and wait for other to provide ideas.

May be we can break this question out to an issue and see if anyone else or the team has input.

@taooceros
Copy link
Member Author

taooceros commented Nov 11, 2020

Sure!

@Flow-Launcher Flow-Launcher deleted a comment from taooceros Nov 11, 2020
jjw24
jjw24 reviewed Nov 11, 2020
View reviewed changes
@jjw24
Copy link
Member

jjw24 commented Nov 11, 2020

Got a question above and also need to resolve conflict, after that this is good to go in 👍

@taooceros
Copy link
Member Author

taooceros commented Nov 11, 2020

Got a question above and also need to resolve conflict, after that this is good to go in 👍

😀😀😉

jjw24
jjw24 previously approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks @taooceros , merging in

@jjw24 jjw24 removed the review in progress Indicates that a review is in progress for this PR label Nov 11, 2020
@jjw24 jjw24 merged commit 54ebe68 into Flow-Launcher:dev Nov 11, 2020
@jjw24 jjw24 mentioned this pull request Nov 11, 2020
Merged
@taooceros taooceros deleted the ProgramPluginImprovement branch November 30, 2020 02:47
@taooceros taooceros linked an issue Dec 9, 2020 that may be closed by this pull request
Merge performance improvements from Wox #174
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjw24 jjw24 jjw24 approved these changes

Assignees

@taooceros taooceros

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Merge performance improvements from Wox

2 participants

@taooceros @jjw24