Repository: nvim-lua/plenary.nvim (lua)

Issue:
plenary.scandir respect_gitignore doesn't work with more complex gitignores
For example:
- `**/bin`
- `!bin/asdf`

both currently will result in unexpected behavior

Pull request:
This PR combines a handful of small fixes and features for the `scandir` module as well as a refactor of the `respect_gitignore` functionality. Instead of attempting to interpret the `.gitignore` file, which is quite difficult when the file contains more advanced patterns, it instead checks relative file paths against the output of `git ls-tree -rt HEAD --name-only`. This should close #76. 

I've split up the changes into separate commits for easier review but will squash them whenever this PR is ready to merge.

Patch:
diff --git a/lua/plenary/scandir.lua b/lua/plenary/scandir.lua
index 05a372d4..a4df4d6b 100644
--- a/lua/plenary/scandir.lua
+++ b/lua/plenary/scandir.lua
@@ -6,21 +6,34 @@ local uv = vim.loop
 
 local m = {}
 
-local get_gitignore = function(basepath)
-  local gitignore = {}
+local make_gitignore = function(basepath)
+  local patterns = {}
   local valid = false
   for _, v in ipairs(basepath) do
     local p = Path:new(v .. os_sep .. ".gitignore")
     if p:exists() then
       valid = true
-      gitignore[v] = {}
+      patterns[v] = { ignored = {}, negated = {} }
       for l in p:iter() do
-        if l ~= "" then
-          local el = l:gsub("%#.*", "")
+        local prefix = l:sub(1, 1)
+        local negated = prefix == "!"
+        if negated then
+          l = l:sub(2)
+          prefix = l:sub(1, 1)
+        end
+        if prefix == "/" then
+          l = v .. l
+        end
+        if not (prefix == "" or prefix == "#") then
+          local el = vim.trim(l)
+          el = el:gsub("%-", "%%-")
           el = el:gsub("%.", "%%.")
-          el = el:gsub("%*", "%.%*")
+          el = el:gsub("/%*%*/", "/%%w+/")
+          el = el:gsub("%*%*", "")
+          el = el:gsub("%*", "%%w+")
+          el = el:gsub("%?", "%%w")
           if el ~= "" then
-            table.insert(gitignore[v], el)
+            table.insert(negated and patterns[v].negated or patterns[v].ignored, el)
           end
         end
       end
@@ -29,21 +42,29 @@ local get_gitignore = function(basepath)
   if not valid then
     return nil
   end
-  return gitignore
-end
-
-local interpret_gitignore = function(gitignore, bp, entry)
-  for _, v in ipairs(bp) do
-    if entry:find(v, 1, true) then
-      for _, w in ipairs(gitignore[v]) do
-        if entry:match(w) then
-          return false
+  return function(bp, entry)
+    for _, v in ipairs(bp) do
+      if entry:find(v, 1, true) then
+        local negated = false
+        for _, w in ipairs(patterns[v].ignored) do
+          if not negated and entry:match(w) then
+            for _, inverse in ipairs(patterns[v].negated) do
+              if not negated and entry:match(inverse) then
+                negated = true
+              end
+            end
+            if not negated then
+              return false
+            end
+          end
         end
       end
     end
+    return true
   end
-  return true
 end
+-- exposed for testing
+m.__make_gitignore = make_gitignore
 
 local handle_depth = function(base_paths, entry, depth)
   for _, v in ipairs(base_paths) do
@@ -73,6 +94,8 @@ local gen_search_pat = function(pattern)
       end
       return false
     end
+  elseif type(pattern) == "function" then
+    return pattern
   end
 end
 
@@ -85,8 +108,8 @@ local process_item = function(opts, name, typ, current_dir, next_dir, bp, data,
       else
         table.insert(next_dir, entry)
       end
-      if opts.add_dirs then
-        if not giti or interpret_gitignore(giti, bp, entry .. "/") then
+      if opts.add_dirs or opts.only_dirs then
+        if not giti or giti(bp, entry .. "/") then
           if not msp or msp(entry) then
             table.insert(data, entry)
             if opts.on_insert then
@@ -95,9 +118,9 @@ local process_item = function(opts, name, typ, current_dir, next_dir, bp, data,
           end
         end
       end
-    else
+    elseif not opts.only_dirs then
       local entry = current_dir .. os_sep .. name
-      if not giti or interpret_gitignore(giti, bp, entry) then
+      if not giti or giti(bp, entry) then
         if not msp or msp(entry) then
           table.insert(data, entry)
           if opts.on_insert then
@@ -117,9 +140,10 @@ end
 -- @param opts: table to change behavior
 --   opts.hidden (bool):              if true hidden files will be added
 --   opts.add_dirs (bool):            if true dirs will also be added to the results
+--   opts.only_dirs (bool):           if true only dirs will be added to the results
 --   opts.respect_gitignore (bool):   if true will only add files that are not ignored by the git (uses each gitignore found in path table)
 --   opts.depth (int):                depth on how deep the search should go
---   opts.search_pattern (regex):     regex for which files will be added, string or table of strings
+--   opts.search_pattern (regex):     regex for which files will be added, string, table of strings, or callback (should return bool)
 --   opts.on_insert(entry):           Will be called for each element
 --   opts.silent (bool):              if true will not echo messages that are not accessible
 -- @return array with files
@@ -130,8 +154,8 @@ m.scan_dir = function(path, opts)
   local base_paths = vim.tbl_flatten { path }
   local next_dir = vim.tbl_flatten { path }
 
-  local gitignore = opts.respect_gitignore and get_gitignore(base_paths) or nil
-  local match_seach_pat = opts.search_pattern and gen_search_pat(opts.search_pattern) or nil
+  local gitignore = opts.respect_gitignore and make_gitignore(base_paths) or nil
+  local match_search_pat = opts.search_pattern and gen_search_pat(opts.search_pattern) or nil
 
   for i = table.getn(base_paths), 1, -1 do
     if uv.fs_access(base_paths[i], "X") == false then
@@ -154,7 +178,7 @@ m.scan_dir = function(path, opts)
         if name == nil then
           break
         end
-        process_item(opts, name, typ, current_dir, next_dir, base_paths, data, gitignore, match_seach_pat)
+        process_item(opts, name, typ, current_dir, next_dir, base_paths, data, gitignore, match_search_pat)
       end
     end
   until table.getn(next_dir) == 0
@@ -169,9 +193,10 @@ end
 -- @param opts: table to change behavior
 --   opts.hidden (bool):              if true hidden files will be added
 --   opts.add_dirs (bool):            if true dirs will also be added to the results
+--   opts.only_dirs (bool):           if true only dirs will be added to the results
 --   opts.respect_gitignore (bool):   if true will only add files that are not ignored by git
 --   opts.depth (int):                depth on how deep the search should go
---   opts.search_pattern (lua regex): depth on how deep the search should go
+--   opts.search_pattern (regex):     regex for which files will be added, string, table of strings, or callback (should return bool)
 --   opts.on_insert function(entry):  will be called for each element
 --   opts.on_exit function(results):  will be called at the end
 --   opts.silent (bool):              if true will not echo messages that are not accessible
@@ -184,8 +209,8 @@ m.scan_dir_async = function(path, opts)
   local current_dir = table.remove(next_dir, 1)
 
   -- TODO(conni2461): get gitignore is not async
-  local gitignore = opts.respect_gitignore and get_gitignore() or nil
-  local match_seach_pat = opts.search_pattern and gen_search_pat(opts.search_pattern) or nil
+  local gitignore = opts.respect_gitignore and make_gitignore(base_paths) or nil
+  local match_search_pat = opts.search_pattern and gen_search_pat(opts.search_pattern) or nil
 
   -- TODO(conni2461): is not async. Shouldn't be that big of a problem but still
   -- Maybe obers async pr can take me out of callback hell
@@ -209,7 +234,7 @@ m.scan_dir_async = function(path, opts)
         if name == nil then
           break
         end
-        process_item(opts, name, typ, current_dir, next_dir, base_paths, data, gitignore, match_seach_pat)
+        process_item(opts, name, typ, current_dir, next_dir, base_paths, data, gitignore, match_search_pat)
       end
       if table.getn(next_dir) == 0 then
         if opts.on_exit then


Test patch:
diff --git a/tests/plenary/scandir_spec.lua b/tests/plenary/scandir_spec.lua
index 0059e452..0e30d5ca 100644
--- a/tests/plenary/scandir_spec.lua
+++ b/tests/plenary/scandir_spec.lua
@@ -1,4 +1,6 @@
 local scan = require "plenary.scandir"
+local mock = require "luassert.mock"
+local stub = require "luassert.stub"
 local eq = assert.are.same
 
 local contains = function(tbl, str)
@@ -73,6 +75,16 @@ describe("scandir", function()
       eq(false, contains(dirs, "./asdf/asdf/adsf.lua"))
     end)
 
+    it("with only directories", function()
+      local dirs = scan.scan_dir(".", { only_dirs = true })
+      eq("table", type(dirs))
+      eq(false, contains(dirs, "./CHANGELOG.md"))
+      eq(false, contains(dirs, "./lua/plenary/job.lua"))
+      eq(true, contains(dirs, "./lua"))
+      eq(true, contains(dirs, "./tests"))
+      eq(false, contains(dirs, "./asdf/asdf/adsf.lua"))
+    end)
+
     it("until depth 1 is reached", function()
       local dirs = scan.scan_dir(".", { depth = 1 })
       eq("table", type(dirs))
@@ -127,6 +139,86 @@ describe("scandir", function()
       eq(true, contains(dirs, "./data/plenary/filetypes/builtin.lua"))
       eq(false, contains(dirs, "./README.md"))
     end)
+
+    it("with callback search pattern", function()
+      local dirs = scan.scan_dir(".", {
+        search_pattern = function(entry)
+          return entry:match "filetype"
+        end,
+      })
+      eq("table", type(dirs))
+      eq(true, contains(dirs, "./scripts/update_filetypes_from_github.lua"))
+      eq(true, contains(dirs, "./lua/plenary/filetype.lua"))
+      eq(true, contains(dirs, "./tests/plenary/filetype_spec.lua"))
+      eq(true, contains(dirs, "./data/plenary/filetypes/base.lua"))
+      eq(true, contains(dirs, "./data/plenary/filetypes/builtin.lua"))
+      eq(false, contains(dirs, "./README.md"))
+    end)
+  end)
+
+  describe("gitignore", function()
+    local Path = require "plenary.path"
+    local mock_path, mock_gitignore
+    before_each(function()
+      mock_path = {
+        exists = stub.new().returns(true),
+        iter = function()
+          local i = 0
+          local n = table.getn(mock_gitignore)
+          return function()
+            i = i + 1
+            if i <= n then
+              return mock_gitignore[i]
+            end
+          end
+        end,
+      }
+      Path.new = stub.new().returns(mock_path)
+    end)
+    after_each(function()
+      Path.new:revert()
+    end)
+
+    describe("ignores path", function()
+      it("when path matches pattern exactly", function()
+        mock_gitignore = { "ignored.txt" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(false, should_add({ "path" }, "./path/ignored.txt"))
+      end)
+      it("when path matches * pattern", function()
+        mock_gitignore = { "*.txt" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(false, should_add({ "path" }, "./path/dir/ignored.txt"))
+      end)
+      it("when path matches leading ** pattern", function()
+        mock_gitignore = { "**/ignored.txt" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(false, should_add({ "path" }, "./path/dir/subdir/ignored.txt"))
+      end)
+      it("when path matches trailing ** pattern", function()
+        mock_gitignore = { "/dir/**" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(false, should_add({ "path" }, "./path/dir/subdir/ignored.txt"))
+      end)
+      it("when path matches ? pattern", function()
+        mock_gitignore = { "ignore?.txt" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(false, should_add({ "path" }, "./path/ignored.txt"))
+      end)
+    end)
+
+    describe("does not ignore path", function()
+      it("when path does not match", function()
+        mock_gitignore = { "ignored.txt" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(true, should_add({ "path" }, "./path/ok.txt"))
+      end)
+      it("when path is negated", function()
+        mock_gitignore = { "*.txt", "!ok.txt" }
+        local should_add = scan.__make_gitignore { "path" }
+        eq(true, should_add({ "path" }, "./path/ok.txt"))
+      end)
+    end)
   end)
 
   describe("ls", function()