ThePrimeagen/harpoon

feat: support async tasks in create_list_item

Opened this issue · 1 comments

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 but create_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, and prepend_async then dealing with when each errors, or what version of create_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:

  1. Add a pre-constructed item to the list: append(something)
  2. 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