-
-
Notifications
You must be signed in to change notification settings - Fork 625
feat: async rename and create file #1870
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,32 +5,28 @@ local core = require "nvim-tree.core" | |||||||||||||||
local notify = require "nvim-tree.notify" | ||||||||||||||||
|
||||||||||||||||
local find_file = require("nvim-tree.actions.finders.find-file").fn | ||||||||||||||||
local async = require "nvim-tree.async" | ||||||||||||||||
|
||||||||||||||||
local M = {} | ||||||||||||||||
|
||||||||||||||||
---@async | ||||||||||||||||
local function create_and_notify(file) | ||||||||||||||||
local ok, fd = pcall(vim.loop.fs_open, file, "w", 420) | ||||||||||||||||
if not ok then | ||||||||||||||||
local fd, err | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
err, fd = async.call(vim.loop.fs_open, file, "w", 420) | ||||||||||||||||
else | ||||||||||||||||
fd, err = vim.loop.fs_open(file, "w", 420) | ||||||||||||||||
end | ||||||||||||||||
if err then | ||||||||||||||||
notify.error("Couldn't create file " .. file) | ||||||||||||||||
return | ||||||||||||||||
end | ||||||||||||||||
vim.loop.fs_close(fd) | ||||||||||||||||
events._dispatch_file_created(file) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
local function create_file(file) | ||||||||||||||||
if utils.file_exists(file) then | ||||||||||||||||
local prompt_select = "Overwrite " .. file .. " ?" | ||||||||||||||||
local prompt_input = prompt_select .. " y/n: " | ||||||||||||||||
lib.prompt(prompt_input, prompt_select, { "y", "n" }, { "Yes", "No" }, function(item_short) | ||||||||||||||||
utils.clear_prompt() | ||||||||||||||||
if item_short == "y" then | ||||||||||||||||
create_and_notify(file) | ||||||||||||||||
end | ||||||||||||||||
end) | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
async.call(vim.loop.fs_close, fd) | ||||||||||||||||
else | ||||||||||||||||
create_and_notify(file) | ||||||||||||||||
vim.loop.fs_close(fd) | ||||||||||||||||
end | ||||||||||||||||
events._dispatch_file_created(file) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
local function get_num_nodes(iter) | ||||||||||||||||
|
@@ -41,6 +37,73 @@ local function get_num_nodes(iter) | |||||||||||||||
return i | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
local function create_file(new_file_path) | ||||||||||||||||
-- create a folder for each path element if the folder does not exist | ||||||||||||||||
-- if the answer ends with a /, create a file for the last path element | ||||||||||||||||
local is_last_path_file = not new_file_path:match(utils.path_separator .. "$") | ||||||||||||||||
local path_to_create = "" | ||||||||||||||||
local idx = 0 | ||||||||||||||||
|
||||||||||||||||
local num_nodes = get_num_nodes(utils.path_split(utils.path_remove_trailing(new_file_path))) | ||||||||||||||||
yioneko marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
local is_error = false | ||||||||||||||||
for path in utils.path_split(new_file_path) do | ||||||||||||||||
idx = idx + 1 | ||||||||||||||||
local p = utils.path_remove_trailing(path) | ||||||||||||||||
if #path_to_create == 0 and utils.is_windows then | ||||||||||||||||
path_to_create = utils.path_join { p, path_to_create } | ||||||||||||||||
else | ||||||||||||||||
path_to_create = utils.path_join { path_to_create, p } | ||||||||||||||||
end | ||||||||||||||||
if is_last_path_file and idx == num_nodes then | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
async.schedule() | ||||||||||||||||
end | ||||||||||||||||
if utils.file_exists(new_file_path) then | ||||||||||||||||
local prompt_select = "Overwrite " .. new_file_path .. " ?" | ||||||||||||||||
local prompt_input = prompt_select .. " y/n: " | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
local item_short = async.call(lib.prompt, prompt_input, prompt_select, { "y", "n" }, { "Yes", "No" }) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to make the prompt async? Only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is mandatory in async version. In fact, You can test it with overriding local original = vim.ui.input
vim.ui.input = function(opts, on_confirm)
original(opts, function(input)
vim.defer_fn(function() on_confirm(input) end, 0)
end)
end There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In principle, all callback-style functions should be wrapped to make the async context consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I still don't understand. Yes, the vim.ui.select calls themselves are asynchronous, however the callbacks appear to be executed on the main thread / context. function M.prompt(prompt_input, prompt_select, items_short, items_long, callback)
...
if M.select_prompts then
vim.ui.select(items_short, { prompt = prompt_select, format_item = format_item }, function(item_short)
log.line("dev", "prompt select in_fast_event=%s is_thread=%s", vim.in_fast_event(), vim.is_thread())
callback(item_short)
end)
else
vim.ui.input({ prompt = prompt_input }, function(item_short)
log.line("dev", "prompt input in_fast_event=%s is_thread=%s", vim.in_fast_event(), vim.is_thread())
callback(item_short)
end)
end
end
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because it might be called in main thread (not in async thread), we need to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would you mind I make a simple guidance for understanding of current basic async implementation? I've seen that there are plenty of misunderstandings between us and possibly with future contributors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please. A detailed explaination would be gratefully appreciated. The outcome of this PR:
Unless there is a very good reason, inputs should not be asynchronous. This adds unnecessary complexity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for that I'm gonna to be off-topic, I think the referred async input codepath would not be ever reached, as we already checked the existence of file before nvim-tree.lua/lua/nvim-tree/actions/fs/create-file.lua Lines 136 to 142 in c71f47a
Do you think it's safe to remove this? I just cp from the original implementation. There is no other place with async call to user input. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see... that's a good catch! It does look like this This is existing behaviour: we can safely remove the Please raise a separate PR to remove this dead code path. We will merge that first, keeping our changes small and atomic and easily revertable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found that I referred the wrong variable, |
||||||||||||||||
utils.clear_prompt() | ||||||||||||||||
if item_short == "y" then | ||||||||||||||||
create_and_notify(new_file_path) | ||||||||||||||||
end | ||||||||||||||||
else | ||||||||||||||||
lib.prompt(prompt_input, prompt_select, { "y", "n" }, { "Yes", "No" }, function(item_short) | ||||||||||||||||
utils.clear_prompt() | ||||||||||||||||
if item_short == "y" then | ||||||||||||||||
create_and_notify(new_file_path) | ||||||||||||||||
end | ||||||||||||||||
end) | ||||||||||||||||
end | ||||||||||||||||
else | ||||||||||||||||
create_and_notify(new_file_path) | ||||||||||||||||
end | ||||||||||||||||
elseif not utils.file_exists(path_to_create) then | ||||||||||||||||
local err | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
err = async.call(vim.loop.fs_mkdir, path_to_create, 493) | ||||||||||||||||
else | ||||||||||||||||
local _ | ||||||||||||||||
_, err = vim.loop.fs_mkdir(path_to_create, 493) | ||||||||||||||||
end | ||||||||||||||||
if err then | ||||||||||||||||
notify.error("Could not create folder " .. path_to_create .. ": " .. err) | ||||||||||||||||
is_error = true | ||||||||||||||||
break | ||||||||||||||||
end | ||||||||||||||||
events._dispatch_folder_created(new_file_path) | ||||||||||||||||
end | ||||||||||||||||
end | ||||||||||||||||
if not is_error then | ||||||||||||||||
notify.info(new_file_path .. " was properly created") | ||||||||||||||||
end | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
async.schedule() | ||||||||||||||||
end | ||||||||||||||||
-- synchronously refreshes as we can't wait for the watchers | ||||||||||||||||
find_file(utils.path_remove_trailing(new_file_path)) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
local function get_containing_folder(node) | ||||||||||||||||
if node.nodes ~= nil then | ||||||||||||||||
return utils.path_add_trailing(node.absolute_path) | ||||||||||||||||
|
@@ -49,7 +112,8 @@ local function get_containing_folder(node) | |||||||||||||||
return node.absolute_path:sub(0, -node_name_size - 1) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
function M.fn(node) | ||||||||||||||||
--TODO: once async feature is finalized, use `async.wrap` instead of cb param | ||||||||||||||||
function M.fn(node, cb) | ||||||||||||||||
node = node and lib.get_last_group_node(node) | ||||||||||||||||
if not node or node.name == ".." then | ||||||||||||||||
node = { | ||||||||||||||||
|
@@ -74,45 +138,21 @@ function M.fn(node) | |||||||||||||||
return | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
-- create a folder for each path element if the folder does not exist | ||||||||||||||||
-- if the answer ends with a /, create a file for the last path element | ||||||||||||||||
local is_last_path_file = not new_file_path:match(utils.path_separator .. "$") | ||||||||||||||||
local path_to_create = "" | ||||||||||||||||
local idx = 0 | ||||||||||||||||
|
||||||||||||||||
local num_nodes = get_num_nodes(utils.path_split(utils.path_remove_trailing(new_file_path))) | ||||||||||||||||
local is_error = false | ||||||||||||||||
for path in utils.path_split(new_file_path) do | ||||||||||||||||
idx = idx + 1 | ||||||||||||||||
local p = utils.path_remove_trailing(path) | ||||||||||||||||
if #path_to_create == 0 and vim.fn.has "win32" == 1 then | ||||||||||||||||
path_to_create = utils.path_join { p, path_to_create } | ||||||||||||||||
else | ||||||||||||||||
path_to_create = utils.path_join { path_to_create, p } | ||||||||||||||||
end | ||||||||||||||||
if is_last_path_file and idx == num_nodes then | ||||||||||||||||
create_file(path_to_create) | ||||||||||||||||
elseif not utils.file_exists(path_to_create) then | ||||||||||||||||
local success = vim.loop.fs_mkdir(path_to_create, 493) | ||||||||||||||||
if not success then | ||||||||||||||||
notify.error("Could not create folder " .. path_to_create) | ||||||||||||||||
is_error = true | ||||||||||||||||
break | ||||||||||||||||
if M.enable_async then | ||||||||||||||||
async.exec(create_file, new_file_path, function(err) | ||||||||||||||||
if cb then | ||||||||||||||||
cb(err) | ||||||||||||||||
end | ||||||||||||||||
events._dispatch_folder_created(new_file_path) | ||||||||||||||||
end | ||||||||||||||||
end | ||||||||||||||||
if not is_error then | ||||||||||||||||
notify.info(new_file_path .. " was properly created") | ||||||||||||||||
end) | ||||||||||||||||
else | ||||||||||||||||
create_file(new_file_path) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
-- synchronously refreshes as we can't wait for the watchers | ||||||||||||||||
find_file(utils.path_remove_trailing(new_file_path)) | ||||||||||||||||
end) | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
function M.setup(opts) | ||||||||||||||||
M.enable_reload = not opts.filesystem_watchers.enable | ||||||||||||||||
M.enable_async = opts.experimental.async.create_file | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
return M |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
---Idea taken from: https://github.com/ms-jpq/lua-async-await | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a lot of unused codepaths in here. Let's trim it down. If we do need functionality we can refer to the source. It looks like: |
||
local co = coroutine | ||
|
||
local M = {} | ||
|
||
---@type table<thread, boolean> | ||
local async_threads = {} | ||
|
||
---Execuate an asynchronous function | ||
---@param func function | ||
---@param ... any The arguments passed to `func`, plus a callback receives (err, ...result) | ||
function M.exec(func, ...) | ||
local nargs = select("#", ...) | ||
local args = { ... } | ||
---@type function | ||
local cb = function() end | ||
if nargs > 0 then | ||
cb = args[nargs] | ||
args[nargs] = nil | ||
end | ||
|
||
local thread = co.create(func) | ||
async_threads[thread] = true | ||
|
||
local function step(...) | ||
local res = { co.resume(thread, ...) } | ||
local ok = table.remove(res, 1) | ||
local err_or_next = res[1] | ||
if co.status(thread) ~= "dead" then | ||
local _, err = xpcall(err_or_next, debug.traceback, step) | ||
alex-courtis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err then | ||
cb(err) | ||
end | ||
else | ||
async_threads[thread] = nil | ||
if ok then | ||
cb(nil, unpack(res)) | ||
else | ||
cb(debug.traceback(thread, err_or_next)) | ||
alex-courtis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
end | ||
end | ||
|
||
step(unpack(args)) | ||
end | ||
|
||
---Test whether we are in async context | ||
---@return boolean | ||
function M.in_async() | ||
local thread = co.running() | ||
return async_threads[thread] ~= nil | ||
end | ||
|
||
---Asynchronously call a function, which has callback as the last parameter (like luv apis) | ||
---@param func function | ||
---@param ... any | ||
---@return any | ||
function M.call(func, ...) | ||
local args = { ... } | ||
return co.yield(function(cb) | ||
table.insert(args, cb) | ||
func(unpack(args)) | ||
end) | ||
end | ||
|
||
---Asynchronous `vim.schedule` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to do this? Please add comments here and at call sites as to why. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As documented in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. So this schedule is essentially a thread join. I do not understand the timing; surely it should be called immediately after the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Certainly we could do that for every callback, but usually this is only needed when the callback contains vim apis (or we are uncertain about this, such as user provided callback). In some cases we may not want to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. In that case, please move the calls to schedule immediately after the calls that require this join. It should be clear exactly when and where it is required. |
||
---See `:h lua-loop-callbacks` and `:h api-fast`. Usually this should be used before `vim.api.*` and `vim.fn.*` calls. | ||
function M.schedule() | ||
return co.yield(function(cb) | ||
vim.schedule(cb) | ||
end) | ||
end | ||
|
||
return M |
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.
TODO @alex-courtis expand on these and think about how to present them in the readme/wiki etc.