Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork628
feat(#3119): Expose filter functions to the api#3121
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Nice work! I'll get to a proper review / test in the next couple of days. |
Apologies for the delay, my time is limited right now. This deserves considerable review and test time which I hope to have in the next few days. |
| @@ -23,7 +23,7 @@ local function search(search_dir, input_path) | |||
| local function iter(dir) | |||
| local realpath, path, name, stat, handle, _ | |||
| local filter_status =explorer.filters:prepare() | |||
| explorer.filters:prepare() | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Great clean up! All the state is in filters now.
| ---@field private explorer Explorer | ||
| ---@field private exclude_list string[] filters.exclude | ||
| ---@field private ignore_list table<string, boolean> filters.custom string table | ||
| ---@field private custom_function (fun(absolute_path: string): boolean)|nil filters.custom function | ||
| ---@field protected status FilterStatus | ||
| ---@field private filter_cache table<string, FILTER_REASON> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| ---@fieldprivatefilter_cache table<string,FILTER_REASON> | |
| ---@fieldprivatefilter_cache table<string,FILTER_REASON> | |
| ---@fieldprivatecustom fun(self:Filters,path:string):boolean | |
| ---@fieldprivatedotfile fun(self:Filters,path:string):boolean | |
| ---@fieldprivategit_ignored fun(self:Filters,path:string):boolean | |
| ---@fieldprivategit_clean fun(self:Filters,path:string):boolean | |
| ---@fieldprivatebuf fun(self:Filters,path:string):boolean | |
| ---@fieldprivatebookmark fun(self:Filters,path:string):boolean |
This is annoying, but it will keep the language server from complaining about inject-field
You could even wrap them in the constructor, although I'm not sure how clean that would look.
It's working nicely with a variety of composed filters and tests on Unfortunately this one's not updating until we manually refresh: filters= {enable=true,git_ignored=false,dotfiles=false,git_clean=false,no_buffer=false,no_bookmark=false,custom=function(path)returnpath:match(".*Makefile")andnotapi.filters.no_buffer(path)end,exclude= {},
Possible solution: fire the event when the user has a custom function. That's not the best for performance, happy for any other ideas. |
This wasn't you, but we need to change the description of the default mapping Hidden->Custom vim.keymap.set("n","U",api.tree.toggle_custom_filter,opts("Toggle Filter: Hidden")) |
| ---@param self Filters | ||
| ---@param path string | ||
| ---@return FILTER_REASON | ||
| ---@diagnostic disable:invisible |
alex-courtisMay 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I've had no success in resolving it. I'll keep at it...
Edit: we can change it to a method so that we may "legally" reference self. This keeps the language server happy:
--- Cache filter function results so subsequent calls to the same path in a loop--- iteration are as fast as possible---@private---@paramfnfun(self:Filters,path:string):boolean---@paramreasonFILTER_REASON---@returnfun(self:Filters,path:string):booleanfunctionFilters:cache_wrapper(fn,reason)---@parampathstring---@returnbooleanlocalfunctioninner(_,path)ifself.filter_cache[reason]==nilthenself.filter_cache[reason]= {}endifself.filter_cache[reason][path]==nilthenself.filter_cache[reason][path]=fn(self,path)endreturnself.filter_cache[reason][path]endreturninnerend
We'll need to move the assignments into the constructor:
self.bookmark=self:cache_wrapper(Filters.bookmark,FILTER_REASON.bookmark)self.custom=self:cache_wrapper(Filters.custom,FILTER_REASON.custom)self.dotfile=self:cache_wrapper(Filters.dotfile,FILTER_REASON.dotfile)self.git_ignored=self:cache_wrapper(Filters.git_ignored,FILTER_REASON.git_ignore)self.git_clean=self:cache_wrapper(Filters.git_clean,FILTER_REASON.git_clean)self.buf=self:cache_wrapper(Filters.buf,FILTER_REASON.buf)self.bookmark=self:cache_wrapper(Filters.bookmark,FILTER_REASON.bookmark)
| ---@field private explorer Explorer | ||
| ---@field private exclude_list string[] filters.exclude | ||
| ---@field private ignore_list table<string, boolean> filters.custom string table | ||
| ---@field private custom_function (fun(absolute_path: string): boolean)|nil filters.custom function | ||
| ---@field protected status FilterStatus | ||
| ---@field private filter_cache table<string, FILTER_REASON> |
alex-courtisMay 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| ---@fieldprivatefilter_cache table<string,FILTER_REASON> | |
| ---@fieldprivatefilter_cache table<FILTER_REASON,table<string,boolean>> |
I think we can go one step further with the custom function: pass a node instead of a path, allowing the user to see the type, stat, children etc. All of the filters could take a node, for consistency and ease of use, as well as aiding any future builtins. There is extra complexity: we need to supply the user with a sanitised node rather than the actual, as per |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is looking fantastic, very straight-forward for the user.
Please
- resolve annotations suggestions
- update default map description:#3121 (comment)
- consider using
Nodeoverpath:#3121 (comment) - propagate buffer events:#3121 (comment)
Apologies for the volume of comments; this is a significant change.
Thanks for the review! I'll have a chance to go over your comments and make changes within in the next few days. |
Any progress on this one@gracepetryk ? Would you like me to complete this and merge it? No pressure... |
Hi@alex-courtis, I haven't had the time to work on this I thought it would. If you'd like to complete + merge it go ahead :) |
Uh oh!
There was an error while loading.Please reload this page.
Addresses#3119. Filter methods have all been normalized to accept a single
pathvariable so that they may be easily called during a call tocustom. Results are cached based on the path argument during each render loop to avoid computing filters more than once if the filter is enabled and also called incustom.This enables filter configurations that were not possible before, for example combining filters so that items are only filtered if they match both: