harpoon
harpoon copied to clipboard
feat: support async tasks in create_list_item
First, off, just want to say I'm really impressed with the generalizability of harpoon2, I see myself using it a bunch in the future for more 1-button tasks. Good stuff!
What issue are you having that you need harpoon to solve?
Calling append
with no arguments results in create_list_item
being called which seems to be intended as the default/normal new item behavior.
For the default harpoon list (quick file switch) this works well as all the information required to create a new list item is immediately present (current buffer, etc...).
However if that is not the case, it is quite likely that some async function will need to be called, e.g. vim.ui.input
or some telescope picker. In these cases, it currently seems impossible to support no-arg append
.
We can currently side-step the issue by simply not allowing no-arg append for this list and just having some other function that calls append(item)
with the item in the callback of the async task. But it seems non-idiomatic? Most lists will have append()
work fine, while lists that require async tasks to "default create" will have to error when append
is called with no args.
(I ran into this trying to make a harpoon list for overseer.nvim where I would like the no-arg append to open the new shell task prompt from :OverseerRunCmd
, implementation here: https://github.com/itsfrank/overseer-quick-tasks)
Minimal example
Say we want a harpoon list of strings. And in the case of list:append(nil)
we want to prompt the user to input a string. I would try to write something like this:
create_list_item = function(config, obj)
-- item added in the ui menu
if item ~= nil then
return { value = obj }
end
-- append called with no args
vim.ui.input({}, function(s)
-- what do I do with s??
end)
-- what do I return??
-- currently this is what I have to do here
error("don't use append() with no args, instead use my_custom_function()")
end
Why doesn't the current config help?
Currently, list:append()
calls create_list_item
directly and expects an immediate return value. This makes it impossible to run an async task from create_list_item
unless append
itself is called from a coroutine (see below).
This prevents a whole category of implementations for create_list_item
, anything that needs to communicate to something outside the main nvim thread is not possible (e.g. ui/prompts like telescope, executing shell commands, http/lsp requests, etc...)
What proposed api changes are you suggesting?
(update: see my reply below for my current thoughts, the rest of this post can be skipped)
I'm not exactly sure the right solution, I attempted to solve it by using coroutines, but this fails when list.append()
is called from the main thread (virtually all cases).
create_list_item
implementation from example from above leveraging coroutines:
create_list_item = function(_, item)
... -- omitting check from ui
local co = coroutine.running()
vim.ui.input({}, function(s)
coroutine.resume(co, s) -- s gets returned by yield() below
end)
local input = coroutine.yield()
return { -- regular return after the async task
value = input
}
end
For the above to work, a user needs to call it like this:
local co = coroutine.create(function()
harpoon:list("strings"):append()
end)
coroutine.resume(co)
Though asking users to do this would certainly get tedious. I am trying to see if there are some changes that could be made within list:append()
but it currently returning self
makes it difficult. Additionally, I am unsure if any assumptions or invariants are being violated by append
/create_list_item
being potentially async
Other approaches
There are certainly other ways this could be solved, though they would all require some change to the existing API. E.g.:
- A list could be declared as either sync or async and the behavior of these functions would change respectively.
- Or this could be inferred by implementing a new function called something like
create_list_item_async
which has a callback, if this one is set butcreate_list_item
is nil, the list is inferred to be async.
Not sure what the best approaches are, but it seems to be solvable. I'd be glad to implement the solution if an acceptable design is found
Ok, I've been mulling over this for a few days, I think any solution that maintains the current paradigms requires either:
- Making
append
/prepend
async and taking a callback (or returning a promise/future) - Having an additional async version of existing functions like
create_list_item_async
,append_async
, andprepend_async
then dealing with when each errors, or what version ofcreate_list_item
they call.
Both those don't sound very good to me (happy to provide more details on the designs I considered here if that is helpful).
I am starting to think that the current coupling between append
and create_list_item
is likely the issue here. It seems like append
/prepend
are trying to do two different tasks:
- Add a pre-constructed item to the list:
append(something)
- Construct a new item and append it to the list:
apend(nil)
Pretty sure the first has no reason to ever be async, but the second does (see original post above).
Separating the intent of appending an item from that of creating a new one should help make an API that is more flexible/composable, still feels good, and supports async item creation while keeping append
non-async.
This would require a breaking change where append(nil)
is no longer considered valid. And if we want to avoid having two versions of "create item", then the one version we do have will have to be async (via either callback or promise).
append(nil)
might become something like create_new_item(nil, function(item) append(item) end)
(or a future/promise variant).
Hopefully there's an even better solution, and maybe the path forward is to just say that async item creation is not supported by the harpoon API and requires external functions. But if we do want to support async item creation in the harpoon api, I am having trouble seeing how to do it without a breaking change