Skip to content
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

Fuzzy finder #1393

Merged
merged 45 commits into from
Jan 10, 2024
Merged

Fuzzy finder #1393

merged 45 commits into from
Jan 10, 2024

Conversation

colinkiama
Copy link
Member

Continues the work from #1159
Fixes #134

As mentioned in the original pull request, the implementation in this pull request will be good enough to be used as plugin. Once this feature is mature enough then we could consider making it part of the core of the editor.

@colinkiama colinkiama changed the title WIP: Fuzzy finder Fuzzy finder Dec 20, 2023
@colinkiama colinkiama requested a review from a team December 20, 2023 21:51
@colinkiama colinkiama marked this pull request as ready for review December 20, 2023 21:51
@colinkiama colinkiama marked this pull request as draft December 20, 2023 21:51
@colinkiama colinkiama marked this pull request as ready for review December 20, 2023 21:55
@colinkiama
Copy link
Member Author

Regarding the code details:

  • Is it necessary to use a Box with custom styling for the search results? Can a ListBox be used. Need to think about eventual porting to Gtk4. Note that Dialog is deprecated in Gtk4.
  • It is usually easier to use the markup property of labels rather than using AttrList.
  • In prep for Gtk4 try to use EventControllerKey to handler key presses
  • In prep for Gtk4 avoid moving windows or playing with their positioning. That should be left to the window manager
  • There are a couple of new signals in GitManager that it appears nothing connects to - can these be removed.

All of these have been addressed.

Thanks for working on this. It is impressively fast. There are a few comments:

  • I'd like to see a close button on the search dialog. Maybe better would be to insert it into the sidebar so it is always available without knowing a shortcut, but that could be left for another PR (may be easier if the feature is in main code)
  • I do not like <Ctrl>P as the shortcut as that is usually used to show a print dialog. Maybe <Alt>F would be more memorable?
  • Not sure that the dialog should be modal and close after selecting a file - the user may want to find/open more than one file.
  • I think the results should prioritize files that are in the currently active project
  • Some very fuzzy matches are prioritized over what appear to closer matches. For example searching for "Sea" in the Code project gives "spell.vala" as the first result, "Settings.vala" as the second whereas "Searchbar.vala" is only third.
  • The search also shows non-text files which cannot be opened in Code. These should be filtered out or at least not opened

All of these issues have been addressed except being able to open multiple files. I'm still unsure about how to implement that.

@colinkiama
Copy link
Member Author

I ended up changing the dialog that the fuzzy search appears in into a popover:
Fuzzy Search appearing as Gtk.Popover - Showing results for "sea"
Because of this change, the fuzzy search can also be dismissed by clicking outside of the popover.

@jeremypw
Copy link
Collaborator

jeremypw commented Jan 1, 2024

Thanks for the improvements - I'll retest for a while. The muli-file issue can probably be left for another PR. The Files app has the same issue - selecting a matched file closes the search and if the selected file turns out not to be the desired one you have to repeat the whole search again (which could take a while).

A logical solution might be to use <Ctrl>Click to multi select items in the same way as in e.g. a Files view.

jeremypw and others added 8 commits January 4, 2024 18:33
)

* Add Fuzzy Find menuitem to sidebar

* Consistently use "Find Project Files"

* Move SearchProject to separate file

* Fix lint

* Remove menuitem on deactivation of "Find Project Files" plugin
* Associate popover with foldermanager
* Remove deprecated PopoverConstraint
@colinkiama
Copy link
Member Author

The plugin now does indexing in the background

process_initial_indexing_requests_async.end (res);
status = IndexerStatus.IDLE;
}
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extra tab present


status = IndexerStatus.PROCESSING;
debug ("Fuzzy Search - Indexer now processing!");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Blank line after if {} missing

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

This works pretty well now. Lets bank the code - it can be iterated on if needed later.

Thanks for your work on this!

@jeremypw jeremypw merged commit 5b67e6b into master Jan 10, 2024
6 checks passed
@jeremypw jeremypw deleted the fuzzy-finder branch January 10, 2024 15:48
@jeremypw
Copy link
Collaborator

One thing I would query is how fuzzy the search is and the ordering - some of the results appear only remotely related but prioritised. E.g.with the Code and Files projects open, entering "cv" finds "class.svg" in Files and lists it higher than "cv.po" in Files.

Would it be better to allow use of wildcards or regex or require matching at least 3 consecutive characters?

@colinkiama
Copy link
Member Author

I think that has something to do with the fact that when multiple projects are opened, the paths used when calculating the search score in plugins/fuzzy-search/fuzzy-finder.vala has the project name added to the start of them. This code was there before I started working on the plugin:

// plugins/fuzzy-search/fuzzy-finder.vala
if (project_paths.size > 1) {
    project_name = Path.get_basename (project.root_path);
    path_search_result = fuzzy_match (search_str, @"$project_name/$path", cancellable);
} else {
    // ..

Removing this code should fix the issue you're describing.
Also, tweaking the value of CURRENT_PROJECT_PRIORITY_BONUS may help too.

I think that the use of wildcards, regex or requiring at least 3 consecutive characters are solutions to try only if the suggested fixes above don't work.

@jeremypw
Copy link
Collaborator

Yes, as long as the desired matches come first it doesn't really matter if there are remote matches further down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add fuzzy-finder-like way of opening documents within the active project
3 participants