Skip to content

feat(#2654): filters.custom may be a function #2655

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

Merged
merged 21 commits into from
Feb 11, 2024
Merged

feat(#2654): filters.custom may be a function #2655

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b754082
feat(#2654): add `binaries` field to `filters`
dxrcy Jan 29, 2024
e942899
feat(#2648): allow functions in `filters.custom`
dxrcy Jan 29, 2024
be1e6e4
Merge branch 'nvim-tree:master' into master
dxrcy Jan 30, 2024
d65092d
ci: fix: stylua check
dxrcy Jan 30, 2024
0e46599
ci: fix: add new keybind and config to docs
dxrcy Jan 30, 2024
1304c03
fix: replace os-specific binary filter with `vim.fn.executable`
dxrcy Feb 3, 2024
89653f3
fix: remove function and mapping for `binaries` filter
dxrcy Feb 3, 2024
7978157
fix: add `node` parameter to custom filter function
dxrcy Feb 4, 2024
7339881
fix: update doc for custom filter function signature
dxrcy Feb 4, 2024
0e880dc
fix: add custom filter to `ACCEPTED_TYPES`
dxrcy Feb 4, 2024
2e4a1d1
fix: accept single function for custom filter
dxrcy Feb 4, 2024
5e416f3
fix: change custom filter on `ACCEPTED_TYPES`
dxrcy Feb 4, 2024
b447d69
fix: revert to using `path` for custom filter function
dxrcy Feb 5, 2024
38ded48
fix: use `function` type for custom filter
dxrcy Feb 5, 2024
8363ac5
fix: type for custom filter in help
dxrcy Feb 5, 2024
c4b4aae
fix: custom filter single function no longer mutates `M.config.filter…
dxrcy Feb 5, 2024
1e054f3
Merge branch 'nvim-tree:master' into master
dxrcy Feb 5, 2024
340c581
fix: remove dead `if` statement in custom filter
dxrcy Feb 7, 2024
8385e89
Merge branch 'nvim-tree:master' into master
dxrcy Feb 11, 2024
e771143
fix: separate custom filter function from `M.ignore_list`
dxrcy Feb 11, 2024
c3336b5
doc nit
alex-courtis Feb 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/nvim-tree-lua.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ Enabling this is not useful as there is no means yet to persist bookmarks.
Custom list of vim regex for file/directory names that will not be shown.
Backslashes must be escaped e.g. "^\\.git". See |string-match|.
Toggle via |nvim-tree-api.tree.toggle_custom_filter()|, default `U`
Type: {string}, Default: `{}`
Type: {string | function(absolute_path)}, Default: `{}`

*nvim-tree.filters.exclude*
List of directories or files to exclude from filtering: always show them.
Expand Down
3 changes: 3 additions & 0 deletions lua/nvim-tree.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,9 @@ local ACCEPTED_TYPES = {
group_empty = { "boolean", "function" },
root_folder_label = { "function", "string", "boolean" },
},
filters = {
custom = { "function" },
Copy link
Member

Choose a reason for hiding this comment

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

Thank you, looking good!

},
actions = {
open_file = {
window_picker = {
Expand Down
25 changes: 20 additions & 5 deletions lua/nvim-tree/explorer/filters.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,22 @@ local function custom(path)

local basename = utils.path_basename(path)

-- single filter function
if type(M.config.filter_custom) == "function" then
return M.config.filter_custom(path)
end

-- filter custom regexes
local relpath = utils.path_relative(path, vim.loop.cwd())
for pat, _ in pairs(M.ignore_list) do
Copy link
Member

Choose a reason for hiding this comment

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

With some hacking

function M.setup(opts)
  M.config = {
    filter_custom = opts.filters.custom,
    filter_dotfiles = opts.filters.dotfiles,
    filter_git_ignored = opts.filters.git_ignored,
    filter_git_clean = opts.filters.git_clean,
    filter_no_buffer = opts.filters.no_buffer,
    filter_no_bookmark = opts.filters.no_bookmark,
  }

  M.ignore_list = {}
  M.exclude_list = opts.filters.exclude

  log.line("dev", "filters setup M='%s'", vim.inspect(M))
  -- local custom_filter = opts.filters.custom
  -- if custom_filter and #custom_filter > 0 then
  --   for _, filter_name in pairs(custom_filter) do
  --     M.ignore_list[filter_name] = true
  --   end
  -- end
end

I was able to pass the custom function through, however it's never executed as ignore_list is null.

I reckon you'll need to directly call the custom function outside of this loop.

if vim.fn.match(relpath, pat) ~= -1 or vim.fn.match(basename, pat) ~= -1 then
return true
if type(pat) == "function" then
if pat(path) then
Copy link
Member

@alex-courtis alex-courtis Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Let's give the user all the information here to allow creation of any sort of filter.

We can use lib clone_node to render the subtree to pass. That contains executable.

Yes, that's a performance hit, however only for users that use a custom filter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to get the node from the path, at the stage the filter is ran?
I believe the explorer has not yet loaded, so utils.get_node_from_path returns nil.

I am not very familiar with this codebase, so apologies if I am misunderstanding something.

Copy link
Member

Choose a reason for hiding this comment

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

It will often be null; we just have to test for that and ignore it.

return true
end
else
if vim.fn.match(relpath, pat) ~= -1 or vim.fn.match(basename, pat) ~= -1 then
return true
end
end
end

Expand Down Expand Up @@ -153,9 +164,13 @@ function M.setup(opts)
M.exclude_list = opts.filters.exclude

local custom_filter = opts.filters.custom
if custom_filter and #custom_filter > 0 then
for _, filter_name in pairs(custom_filter) do
M.ignore_list[filter_name] = true
if type(custom_filter) == "function" then
M.ignore_list[custom_filter] = true
Copy link
Member

Choose a reason for hiding this comment

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

Should we be using ignore_list at all; can't we just use M.config.custom_filter directly?

Unfortunately I'm out of time until the weekend. I'll give it a solid test then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By M.config.custom_filter, I assume you mean M.config.filter_custom?

It looks like M.config.filter_custom is a boolean, for whether all custom filters are enabled.
Adding the single custom filter function to M.ignore_list would be consistent with adding each item of the custom filter table to it, wouldn't it? As M.ignore_list is a set, with each item checked in the loop in the custom().

Perhaps I should set M.ignore_list to opts.filters.custom (a function), and check the type in custom(), if this is more intuitive or performant.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like M.config.filter_custom is a boolean, for whether all custom filters are enabled.

Right you are, please accept my apologies.

Perhaps I should set M.ignore_list to opts.filters.custom (a function), and check the type in custom(), if this is more intuitive or performant.

That is more intuitive, however it's not good practice to use a member with variable types.

Can we please go one further and explicitly separate, something like

diff --git a/lua/nvim-tree/explorer/filters.lua b/lua/nvim-tree/explorer/filters.lua
index f3beb61..24747c2 100644
--- a/lua/nvim-tree/explorer/filters.lua
+++ b/lua/nvim-tree/explorer/filters.lua
@@ -4,6 +4,7 @@ local marks = require "nvim-tree.marks"
 local M = {
   ignore_list = {},
   exclude_list = {},
+  custom_function = nil,
 }

 ---@param path string
@@ -84,18 +85,17 @@ local function custom(path)

   local basename = utils.path_basename(path)

+  -- filter by user's custom function
+  if M.custom_function and M.custom_function(path) then
+    return true
+  end
+
   -- filter custom regexes
   local relpath = utils.path_relative(path, vim.loop.cwd())
   for pat, _ in pairs(M.ignore_list) do
-    if type(pat) == "function" then
-      if pat(path) then
-        return true
-      end
-    else
       if vim.fn.match(relpath, pat) ~= -1 or vim.fn.match(basename, pat) ~= -1 then
         return true
       end
-    end
   end

   local idx = path:match ".+()%.[^.]+$"
@@ -160,7 +160,7 @@ function M.setup(opts)

   local custom_filter = opts.filters.custom
   if type(custom_filter) == "function" then
-    M.ignore_list[custom_filter] = true
+    M.custom_function = custom_filter
   else
     if custom_filter and #custom_filter > 0 then
       for _, filter_name in pairs(custom_filter) do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That looks like a better solution.
Though this won't let a user define a both a function and a pattern, but I don't think this is a problem, as a custom function can of course contain a pattern check.

custom = function(path)
    if vim.fn.match(path, ".*\\.lock") ~= -1 then -- Could be a specific function like substring
        return true
    end
    return is_binary(path)
end

Previously:

custom = {
    "*.lock",
    function(path) -- Or simply pass `is_binary`
        return is_binary(path)
    end,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to go back to expecting a table<function | string>, instead of a function | table<string>?
A singular function would just be wrapped in a table: custom = { is_binary }.

If this is not preferable, I do not mind.

Copy link
Member

Choose a reason for hiding this comment

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

Though this won't let a user define a both a function and a pattern, but I don't think this is a problem, as a custom function can of course contain a pattern check.

Is it possible to go back to expecting a table<function | string>, instead of a function | table?
A singular function would just be wrapped in a table: custom = { is_binary }.

Sorry, I'd prefer we keep it in the style of the other similar code - one (fast) function only.

else
if custom_filter and #custom_filter > 0 then
for _, filter_name in pairs(custom_filter) do
M.ignore_list[filter_name] = true
end
end
end
end
Expand Down