From f4819d8b4326ce7a6c41ff624e8c3a34eeed7281 Mon Sep 17 00:00:00 2001 From: Steven Arcangeli Date: Sun, 20 Aug 2023 20:56:54 +0000 Subject: [PATCH] refactor: remove cache side effects from adapter.list --- lua/oil/adapters/files.lua | 8 ++--- lua/oil/adapters/ssh.lua | 3 +- lua/oil/adapters/ssh/sshfs.lua | 9 +++--- lua/oil/adapters/test.lua | 59 +++++++++++++++++++++++----------- lua/oil/cache.lua | 11 ++++--- lua/oil/init.lua | 2 +- lua/oil/view.lua | 10 ++++-- tests/mutator_spec.lua | 37 ++++++++++----------- 8 files changed, 86 insertions(+), 53 deletions(-) diff --git a/lua/oil/adapters/files.lua b/lua/oil/adapters/files.lua index b4b43ca..9dc23c0 100644 --- a/lua/oil/adapters/files.lua +++ b/lua/oil/adapters/files.lua @@ -213,7 +213,7 @@ end ---@param url string ---@param column_defs string[] ----@param cb fun(err?: string, fetch_more?: fun()) +---@param cb fun(err?: string, entries?: oil.InternalEntry[], fetch_more?: fun()) M.list = function(url, column_defs, cb) local _, path = util.parse_url(url) assert(path) @@ -234,6 +234,7 @@ M.list = function(url, column_defs, cb) local read_next read_next = function() uv.fs_readdir(fd, function(err, entries) + local internal_entries = {} if err then uv.fs_closedir(fd, function() cb(err) @@ -244,7 +245,7 @@ M.list = function(url, column_defs, cb) if inner_err then cb(inner_err) else - cb(nil, read_next) + cb(nil, internal_entries, read_next) end end) for _, entry in ipairs(entries) do @@ -253,6 +254,7 @@ M.list = function(url, column_defs, cb) if err then poll(meta_err) else + table.insert(internal_entries, cache_entry) local meta = cache_entry[FIELD_META] -- Make sure we always get fs_stat info for links if entry.type == "link" then @@ -266,12 +268,10 @@ M.list = function(url, column_defs, cb) end meta.link = link meta.link_stat = link_stat - cache.store_entry(url, cache_entry) poll() end end) else - cache.store_entry(url, cache_entry) poll() end end diff --git a/lua/oil/adapters/ssh.lua b/lua/oil/adapters/ssh.lua index 05abfe3..fc783c9 100644 --- a/lua/oil/adapters/ssh.lua +++ b/lua/oil/adapters/ssh.lua @@ -1,4 +1,3 @@ -local cache = require("oil.cache") local config = require("oil.config") local constants = require("oil.constants") local fs = require("oil.fs") @@ -206,7 +205,7 @@ end ---@param url string ---@param column_defs string[] ----@param callback fun(err: nil|string, fetch_more?: fun()) +---@param callback fun(err?: string, entries?: oil.InternalEntry[], fetch_more?: fun()) M.list = function(url, column_defs, callback) local res = M.parse_url(url) diff --git a/lua/oil/adapters/ssh/sshfs.lua b/lua/oil/adapters/ssh/sshfs.lua index 6b4bf10..99438c1 100644 --- a/lua/oil/adapters/ssh/sshfs.lua +++ b/lua/oil/adapters/ssh/sshfs.lua @@ -126,7 +126,7 @@ local dir_meta = {} ---@param url string ---@param path string ----@param callback fun(err: nil|string, fetch_more?: fun()) +---@param callback fun(err?: string, entries?: oil.InternalEntry[], fetch_more?: fun()) function SSHFS:list_dir(url, path, callback) local path_postfix = "" if path ~= "" then @@ -145,6 +145,7 @@ function SSHFS:list_dir(url, path, callback) assert(lines) local any_links = false local entries = {} + local cache_entries = {} for _, line in ipairs(lines) do if line ~= "" and not line:match("^total") then local name, type, meta = parse_ls_line(line) @@ -155,9 +156,9 @@ function SSHFS:list_dir(url, path, callback) any_links = true end local cache_entry = cache.create_entry(url, name, type) + table.insert(cache_entries, cache_entry) entries[name] = cache_entry cache_entry[FIELD_META] = meta - cache.store_entry(url, cache_entry) end end end @@ -184,10 +185,10 @@ function SSHFS:list_dir(url, path, callback) end end end - callback() + callback(nil, cache_entries) end) else - callback() + callback(nil, cache_entries) end end) end diff --git a/lua/oil/adapters/test.lua b/lua/oil/adapters/test.lua index a495269..c4176ef 100644 --- a/lua/oil/adapters/test.lua +++ b/lua/oil/adapters/test.lua @@ -1,4 +1,5 @@ local cache = require("oil.cache") +local util = require("oil.util") local M = {} ---@param url string @@ -7,11 +8,49 @@ M.normalize_url = function(url, callback) callback(url) end +local dir_listing = {} + ---@param url string ---@param column_defs string[] ----@param cb fun(err?: string, fetch_more?: fun()) +---@param cb fun(err?: string, entries?: oil.InternalEntry[], fetch_more?: fun()) M.list = function(url, column_defs, cb) - cb() + local _, path = util.parse_url(url) + local entries = dir_listing[path] or {} + local cache_entries = {} + for _, entry in ipairs(entries) do + local cache_entry = cache.create_entry(url, entry.name, entry.entry_type) + table.insert(cache_entries, cache_entry) + end + cb(nil, cache_entries) +end + +M.test_clear = function() + dir_listing = {} +end + +---@param path string +---@param entry_type oil.EntryType +---@return oil.InternalEntry +M.test_set = function(path, entry_type) + if path == "/" then + return {} + end + local parent = vim.fn.fnamemodify(path, ":h") + if parent ~= path then + M.test_set(parent, "directory") + end + parent = util.addslash(parent) + if not dir_listing[parent] then + dir_listing[parent] = {} + end + local name = vim.fn.fnamemodify(path, ":t") + local entry = { + name = name, + entry_type = entry_type, + } + table.insert(dir_listing[parent], entry) + local parent_url = "oil-test://" .. parent + return cache.create_and_store_entry(parent_url, entry.name, entry.entry_type) end ---@param name string @@ -20,22 +59,6 @@ M.get_column = function(name) return nil end ----@param path string ----@param entry_type oil.EntryType -M.test_set = function(path, entry_type) - local parent = vim.fn.fnamemodify(path, ":h") - if parent ~= path then - M.test_set(parent, "directory") - end - local url = "oil-test://" .. path - if cache.get_entry_by_url(url) then - -- Already exists - return - end - local name = vim.fn.fnamemodify(path, ":t") - cache.create_and_store_entry("oil-test://" .. parent, name, entry_type) -end - ---@param bufnr integer ---@return boolean M.is_modifiable = function(bufnr) diff --git a/lua/oil/cache.lua b/lua/oil/cache.lua index 5f0e60c..0f6a27e 100644 --- a/lua/oil/cache.lua +++ b/lua/oil/cache.lua @@ -52,10 +52,7 @@ M.create_entry = function(parent_url, name, type) if entry then return entry end - local id = next_id - next_id = next_id + 1 - _cached_id_fmt = nil - return { id, name, type } + return { nil, name, type } end ---@param parent_url string @@ -68,6 +65,12 @@ M.store_entry = function(parent_url, entry) url_directory[parent_url] = parent end local id = entry[FIELD_ID] + if id == nil then + id = next_id + next_id = next_id + 1 + entry[FIELD_ID] = id + _cached_id_fmt = nil + end local name = entry[FIELD_NAME] parent[name] = entry local tmp_dir = tmp_url_directory[parent_url] diff --git a/lua/oil/init.lua b/lua/oil/init.lua index db25c44..1bc69bd 100644 --- a/lua/oil/init.lua +++ b/lua/oil/init.lua @@ -11,7 +11,7 @@ local M = {} ---@class oil.Adapter ---@field name string The unique name of the adapter (this will be set automatically) ----@field list fun(path: string, column_defs: string[], cb: fun(err: nil|string, fetch_more: nil|fun())) Async function to list a directory. Entries should be stored in the cache. +---@field list fun(path: string, column_defs: string[], cb: fun(err?: string, entries?: oil.InternalEntry[], fetch_more?: fun())) Async function to list a directory. ---@field is_modifiable fun(bufnr: integer): boolean Return true if this directory is modifiable (allows for directories with read-only permissions). ---@field get_column fun(name: string): nil|oil.ColumnDefinition If the adapter has any adapter-specific columns, return them when fetched by name. ---@field normalize_url fun(url: string, callback: fun(url: string)) Before oil opens a url it will be normalized. This allows for link following, path normalizing, and converting an oil file url to the actual path of a file. diff --git a/lua/oil/view.lua b/lua/oil/view.lua index 5012c1d..191262f 100644 --- a/lua/oil/view.lua +++ b/lua/oil/view.lua @@ -579,13 +579,19 @@ M.render_buffer_async = function(bufnr, opts, callback) end cache.begin_update_url(bufname) - adapter.list(bufname, config.columns, function(err, fetch_more) + adapter.list(bufname, config.columns, function(err, entries, fetch_more) loading.set_loading(bufnr, false) if err then cache.end_update_url(bufname) handle_error(err) return - elseif fetch_more then + end + if entries then + for _, entry in ipairs(entries) do + cache.store_entry(bufname, entry) + end + end + if fetch_more then local now = vim.loop.hrtime() / 1e6 local delta = now - start_ms -- If we've been chugging for more than 40ms, go ahead and render what we have diff --git a/tests/mutator_spec.lua b/tests/mutator_spec.lua index 00e8932..38334d3 100644 --- a/tests/mutator_spec.lua +++ b/tests/mutator_spec.lua @@ -22,6 +22,7 @@ a.describe("mutator", function() for _, bufnr in ipairs(vim.api.nvim_list_bufs()) do vim.api.nvim_buf_delete(bufnr, { force = true }) end + test_adapter.test_clear() cache.clear_everything() end) @@ -60,7 +61,7 @@ a.describe("mutator", function() end) it("detects deleted files", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() set_lines(bufnr, {}) @@ -71,7 +72,7 @@ a.describe("mutator", function() end) it("detects deleted directories", function() - local dir = cache.create_and_store_entry("oil-test:///foo/", "bar", "directory") + local dir = test_adapter.test_set("/foo/bar", "directory") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() set_lines(bufnr, {}) @@ -82,7 +83,7 @@ a.describe("mutator", function() end) it("detects deleted links", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "link") + local file = test_adapter.test_set("/foo/a.txt", "link") file[FIELD_META] = { link = "b.txt" } vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() @@ -94,7 +95,7 @@ a.describe("mutator", function() end) it("ignores empty lines", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() local cols = view.format_entry_cols(file, {}, {}, test_adapter) @@ -156,7 +157,7 @@ a.describe("mutator", function() end) it("errors on duplicate names for existing files", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() set_lines(bufnr, { @@ -184,7 +185,7 @@ a.describe("mutator", function() end) it("parses a rename as a delete + new", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() set_lines(bufnr, { @@ -198,8 +199,8 @@ a.describe("mutator", function() end) it("detects renamed files that conflict", function() - local afile = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") - local bfile = cache.create_and_store_entry("oil-test:///foo/", "b.txt", "file") + local afile = test_adapter.test_set("/foo/a.txt", "file") + local bfile = test_adapter.test_set("/foo/b.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() set_lines(bufnr, { @@ -226,7 +227,7 @@ a.describe("mutator", function() end) it("views link targets with trailing slashes as the same", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "mydir", "link") + local file = test_adapter.test_set("/foo/mydir", "link") file[FIELD_META] = { link = "dir/" } vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() @@ -267,7 +268,7 @@ a.describe("mutator", function() end) it("constructs DELETE actions", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() local diffs = { @@ -286,7 +287,7 @@ a.describe("mutator", function() end) it("constructs COPY actions", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() local diffs = { @@ -306,7 +307,7 @@ a.describe("mutator", function() end) it("constructs MOVE actions", function() - local file = cache.create_and_store_entry("oil-test:///foo/", "a.txt", "file") + local file = test_adapter.test_set("/foo/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///foo/" } }) local bufnr = vim.api.nvim_get_current_buf() local diffs = { @@ -327,7 +328,7 @@ a.describe("mutator", function() end) it("correctly orders MOVE + CREATE", function() - local file = cache.create_and_store_entry("oil-test:///", "a.txt", "file") + local file = test_adapter.test_set("/a.txt", "file") vim.cmd.edit({ args = { "oil-test:///" } }) local bufnr = vim.api.nvim_get_current_buf() local diffs = { @@ -354,8 +355,8 @@ a.describe("mutator", function() end) it("resolves MOVE loops", function() - local afile = cache.create_and_store_entry("oil-test:///", "a.txt", "file") - local bfile = cache.create_and_store_entry("oil-test:///", "b.txt", "file") + local afile = test_adapter.test_set("/a.txt", "file") + local bfile = test_adapter.test_set("/b.txt", "file") vim.cmd.edit({ args = { "oil-test:///" } }) local bufnr = vim.api.nvim_get_current_buf() local diffs = { @@ -554,7 +555,7 @@ a.describe("mutator", function() end) a.it("deletes entries", function() - local file = cache.create_and_store_entry("oil-test:///", "a.txt", "file") + local file = test_adapter.test_set("/a.txt", "file") local actions = { { type = "delete", url = "oil-test:///a.txt", entry_type = "file" }, } @@ -568,7 +569,7 @@ a.describe("mutator", function() end) a.it("moves entries", function() - local file = cache.create_and_store_entry("oil-test:///", "a.txt", "file") + local file = test_adapter.test_set("/a.txt", "file") local actions = { { type = "move", @@ -592,7 +593,7 @@ a.describe("mutator", function() end) a.it("copies entries", function() - local file = cache.create_and_store_entry("oil-test:///", "a.txt", "file") + local file = test_adapter.test_set("/a.txt", "file") local actions = { { type = "copy",