linrongbin16 / gitlinker.nvim

Maintained fork of ruifm's gitlinker, refactored with bug fixes, ssh aliases, blame support and other improvements.
GNU General Public License v3.0
163 stars 9 forks source link

Async api #181

Closed lopi-py closed 10 months ago

lopi-py commented 10 months ago

The title says it. It is quite annoying to have the editor to freeze for a second, an async managed api would improve the ux a lot. If you are open to this, I could try to make a PR.

linrongbin16 commented 10 months ago

The title says it. It is quite annoying to have the editor to freeze for a second, an async managed api would improve the ux a lot. If you are open to this, I could try to make a PR.

hi @lopi-py, do you mean, the 'GitLink' command should run without blocking? e.g. don't wait for those git commands finishing?

that looks like a good improvement!

Also I think we can add a timeout config to force terminate the child process. but you can just implement the async running, leave the other things to me.

lopi-py commented 10 months ago

@linrongbin16 Ok, would you mind if I use plenary.nvim for async (and maybe other) utilities? or do you want to keep this plugin dependency-free? I think plenary.nvim is one of that plugins that every neovim user has installed lol.

linrongbin16 commented 10 months ago

@linrongbin16 Ok, would you mind if I use plenary.nvim for async (and maybe other) utilities? or do you want to keep this plugin dependency-free? I think plenary.nvim is one of that plugins that every neovim user has installed lol.

emmmm, in early days when I just forked this plugin, it's depend on plenary, the original author use it to handle child process jobs and path.

but the path seems not working correctly on Windows, so I replaced it with another library I written myself: https://github.com/linrongbin16/commons.nvim.

could you tell me which component are you going to need in plenary?

lopi-py commented 10 months ago

could you tell me which component are you going to need in plenary?

https://github.com/nvim-lua/plenary.nvim/blob/master/lua/plenary/async/init.lua and maybe https://github.com/nvim-lua/plenary.nvim/blob/master/lua/plenary/job.lua

linrongbin16 commented 10 months ago

could you tell me which component are you going to need in plenary?

https://github.com/nvim-lua/plenary.nvim/blob/master/lua/plenary/async/init.lua and maybe https://github.com/nvim-lua/plenary.nvim/blob/master/lua/plenary/job.lua

the commons library I mentioned above, has the jobs like feature, e.g. running child process in either blocking or non-blocking way, just like vim.system api, see: https://linrongbin16.github.io/commons.nvim/#/commons_spawn.

but it doesn't have the async feature. It will have to pass callbacks when running something async.

emmm, actually I don't want to depend on plenary directly again, I am not sure if it support Windows.

maybe I will embed the async source code to the commons library and then use it here.

please let me take a look into it.

lopi-py commented 10 months ago

Well, plenary works fine for me. I use it on Windows with no issues. We could take https://github.com/ms-jpq/lua-async-await as reference.

linrongbin16 commented 10 months ago

Well, plenary works fine for me. I use it on Windows with no issues. We could take https://github.com/ms-jpq/lua-async-await as reference.

it looks good, single file is much easier to embed into the commons library directly.

linrongbin16 commented 10 months ago

I also found below coroutine/async library for Neovim:

https://github.com/yutkat/my-neovim-pluginlist/blob/6793d2e4e2456cba03d17d9352f918a2804ad863/neovim-lua-library.md?plain=1#L68-L79

They look great:

## async

- [nvim-lua/plenary.nvim(async)](https://github.com/nvim-lua/plenary.nvim) ![](https://img.shields.io/github/stars/nvim-lua/plenary.nvim) ![](https://img.shields.io/github/last-commit/nvim-lua/plenary.nvim) ![](https://img.shields.io/github/commit-activity/y/nvim-lua/plenary.nvim)
- [ms-jpq/lua-async-await](https://github.com/ms-jpq/lua-async-await) ![](https://img.shields.io/github/stars/ms-jpq/lua-async-await) ![](https://img.shields.io/github/last-commit/ms-jpq/lua-async-await) ![](https://img.shields.io/github/commit-activity/y/ms-jpq/lua-async-await)
- [notomo/promise.nvim](https://github.com/notomo/promise.nvim) ![](https://img.shields.io/github/stars/notomo/promise.nvim) ![](https://img.shields.io/github/last-commit/notomo/promise.nvim) ![](https://img.shields.io/github/commit-activity/y/notomo/promise.nvim)
- [iamcco/async-await.lua](https://github.com/iamcco/async-await.lua) ![](https://img.shields.io/github/stars/iamcco/async-await.lua) ![](https://img.shields.io/github/last-commit/iamcco/async-await.lua) ![](https://img.shields.io/github/commit-activity/y/iamcco/async-await.lua)
- [kevinhwang91/promise-async](https://github.com/kevinhwang91/promise-async) ![](https://img.shields.io/github/stars/kevinhwang91/promise-async) ![](https://img.shields.io/github/last-commit/kevinhwang91/promise-async) ![](https://img.shields.io/github/commit-activity/y/kevinhwang91/promise-async)
- [lewis6991/async.nvim](https://github.com/lewis6991/async.nvim) ![](https://img.shields.io/github/stars/lewis6991/async.nvim) ![](https://img.shields.io/github/last-commit/lewis6991/async.nvim) ![](https://img.shields.io/github/commit-activity/y/lewis6991/async.nvim)
- [ec965/async-uv.nvim](https://github.com/ec965/async-uv.nvim) ![](https://img.shields.io/github/stars/ec965/async-uv.nvim) ![](https://img.shields.io/github/last-commit/ec965/async-uv.nvim) ![](https://img.shields.io/github/commit-activity/y/ec965/async-uv.nvim)
- [willothy/micro-async.nvim](https://github.com/willothy/micro-async.nvim) ![](https://img.shields.io/github/stars/willothy/micro-async.nvim) ![](https://img.shields.io/github/last-commit/willothy/micro-async.nvim) ![](https://img.shields.io/github/commit-activity/y/willothy/micro-async.nvim)
- [svermeulen/nvim-lusc](https://github.com/svermeulen/nvim-lusc) ![](https://img.shields.io/github/stars/svermeulen/nvim-lusc) ![](https://img.shields.io/github/last-commit/svermeulen/nvim-lusc) ![](https://img.shields.io/github/commit-activity/y/svermeulen/nvim-lusc)
- [nvim-neotest/nvim-nio](https://github.com/nvim-neotest/nvim-nio) ![](https://img.shields.io/github/stars/nvim-neotest/nvim-nio) ![](https://img.shields.io/github/last-commit/nvim-neotest/nvim-nio) ![](https://img.shields.io/github/commit-activity/y/nvim-neotest/nvim-nio)

I hope the library would be: good API design, rich-feature and good performance, cross-platform and less files (so easier to embed to project).

linrongbin16 commented 10 months ago

Well, plenary works fine for me. I use it on Windows with no issues. We could take https://github.com/ms-jpq/lua-async-await as reference.

maybe it's just I'm not familiar with plenary API, so I made mistakes...

anyway, this seems more personal taste....


hi @lopi-py , I do have a question, if you want to run 'GitLink' command async, or say non-blocking. why choose coroutine? I guess we could directly dispatch the lua function to a thread or something?

The luv has a new_work API:

lopi-py commented 10 months ago

That won't work. Its spawning a new thread, which won't take upvalues and we can't pass table as arguments, so we can't share data. This is async work, not multithreading work (also iirrc there is no actual multithreading in neovim). In the neovim world, non-blocking = async

linrongbin16 commented 10 months ago

That won't work. Its spawning a new thread, which won't take upvalues and we can't pass table as arguments, so we can't share data. This is async work, not multithreading work (also iirrc there is no actual multithreading in neovim). In the neovim world, non-blocking = async

Do you mean, the new_work cannot do function closure (e.g. cannot reference outside variables in lua runtime context), only allow passing very limited primitive values (string/integer/boolean/etc) as arguments, correct?


update: oh my bad english, I google translate the word upvalues, the answer should be yes.

Then I agreed with you, the coroutine is worth it.

lopi-py commented 10 months ago

Yep

lopi-py commented 10 months ago

Let's just do async, thats what all the other plugins do.

linrongbin16 commented 10 months ago

Let's just do async, thats what all the other plugins do.

agree

linrongbin16 commented 10 months ago

hi @lopi-py , FYI:

For now the plugin is doing below steps when people type :GitLink browse:

  1. parse the command line arguments to get the router browse, also get the visual selected line numbers. see:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L524

  2. found a binding router function for browser router type. see:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L547

  3. detect what action (for now there's only two actions: clipboard or open in browser) should use, see:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L544

  4. All these things pass to the link function, see:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L356

  5. inside the link function, first run multiple git commands in child process in make_linker function, see:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L120

  6. the make_linker will do (I think that's the thing you want to async):

    1. get git repository root: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L123.
    2. get git branch name: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L129.
    3. get git remote url: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L135.
    4. resolve git host with ssh (if exists): https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L141.
    5. get git commit number: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L151.
    6. get buf file path relative to git repository root directory: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L157.
    7. check whether the commit exists in remote git host: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L164.
    8. check whether the buf file has changed (we will give a warning message using this boolean value): https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L174.
    9. get default branch name (main/master): https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L180.
    10. get current branch name: https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker/linker.lua?plain=1#L181.
  7. invoke the router implementation to generate the remote git host url:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L367

  8. invoke the action function to either copy to clipboard or open in browser:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L384

  9. highlight the selected lines range:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L387

  10. print message to nvim:

    https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L392

BTW, the _url_template_engine function is evaluate the url string template and generate the final git host url:

https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L124

The _merge_routers function is resolving user's router configuration with builtin router configuration (I did some logic there to keep the final router configurations to be more friendly to user):

https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L404

The _router function is to find the binded router implementation for a router type (e.g. browse):

https://github.com/linrongbin16/gitlinker.nvim/blob/585f902062c92bf9fc5135017a5c3c427f8ccfc5/lua/gitlinker.lua?plain=1#L252

lopi-py commented 10 months ago

I have to admit that I ended up using https://github.com/lewis6991/async.nvim as embedded. It's pretty good.

linrongbin16 commented 10 months ago

sure


update: I upgrade the commons library to latest version in https://github.com/linrongbin16/gitlinker.nvim/pull/182. so when you submit PR, there's no other auto-commits.

lopi-py commented 10 months ago

That would conflict I think, because I'm using lewis's async

linrongbin16 commented 10 months ago

That would conflict I think, because I'm using lewis's async

maybe first copy the single file to this project as 'gitlinker.async' module, and use it in other places.

you can also see here: https://github.com/linrongbin16/gitlinker.nvim/blob/1d5947143fac683eb36bbe0d7408620ad6102598/.github/workflows/ci.yml#L42

It can download and copy file when submit a PR. you can add a step to download and copy the async single file to this project.

I need to compare these two library, see which one is better.

lopi-py commented 10 months ago

Ok, I'll add it into gitlinker.async. So you decide what to do

lopi-py commented 10 months ago

Got it working, just some final touches.

linrongbin16 commented 10 months ago

Got it working, just some final touches.

if the PR works, let's just use it in this plugin. It's ok. We don't have to choose 'lua-async-await' library, if there's no other issues or requirements.