nvim-telescope/telescope.nvim

Remove plenary dependency?!

Conni2461 opened this issue · 21 comments

Conni2461 commented

and replace with vim internals (#2529 (comment)) or maybe we want to slim down plenary

clason commented

I think it makes sense to tackle this here one module at a time; replacing a single stale plenary module with a maintained upstream module is a win itself, even if plenary itself remains a dependency.

I should also mention that it's perfectly fine to make feature requests (or PRs) to Neovim if one of the builtin modules is missing something that's needed here (and not too telescope-specific; otherwise just vendoring it here is best).

Of course, slimming down plenary and removing obsolete modules (or refactoring them based on upstream) is appreciated as well! I've heard repeatedly that the (understandable) lack of actual development there has dissuaded someone from contributing there. Something to consider is also to more strongly modularize plenary along the lines of https://github.com/echasnovski/mini.nvim (so each module can be installed independently as well).

From a brief grep of telescope, here's what's used, with possible replacements:

Conni2461 commented
  • plenary.busted / plenary.test_harness -> ?
clason commented

I didn't list that because it's not an actual dependency for using telescope; you can handle this purely on the CI side (as we do in nvim-treesitter).

  • 👍1
Conni2461 commented

I would have done it simiar to plenary.filetype replacement, step by step, especially because i dont have that much time.

I need to evaluate every module because i'm no longer that much up to date with neovim development and i also have to look into mini.nvim

Edit: strings upstreamen would probably be somewhat beneficial because its api-fast and calls c functions linetabsize_col would be way easier to do in nvim core

  • 👍1
clason commented

I need to evaluate every module because i'm no longer that much up to date with neovim development

Of course; This Is The Way. Let me know if you have any questions about the Neovim side (here or on Matrix).

Edit: strings upstreamen would probably be somewhat beneficial because its api-fast and calls c functions linetabsize_col would be way easier to do in nvim core

Yes, that is a good argument for having it in core; also consistent cross-platform handling of multibyte characters would be useful to have.

  • 👍1
max397574 commented

I think plenary.strings could be quite easily upstreamed
it's nearly standalone
the only thing that's required from other modules is the path.sep
which imo could also be upstreamed since it's something you see really often in plugins and configs

I could open a pr for this

clason commented

I don't think plenary.strings is upstreamable as-is. The way these things are done is that we first identify what we want to have in core -- including the API -- and then implement it. Requirements are often quite different for core than for a plugin.

(In particular, as @Conni2461 wrote, core should make use of Neovim's string handling functions in C as much as possible.)

  • 👍2
tjdevries commented

my only concern with the way things are for vim.system right now is I don't think it's as performant for doing line based items (which a lot of operations are for neovim). One thing that would be really awesome for vim.system would be some on_line or lines parameter or similar, and maybe some string wrangling in C to make things really zoom. But not sure, I'd have to do some benchmarks again.

tjdevries commented

Would have really liked popup to have been fully fleshed out and made to match some of the vim apis, since that's a nice two-for-one kind of situation where a bunch of plugins can support vim and nvim with the same functions. But that doesn't look like it's going to happen (haven't seen any progress on that and I don't have any desire to drive those kinds of projects at the moment)

  • 👍1
tjdevries commented

Nice to see that maybe plenary can just be deprecated soon and a lot of the stuff has been upstreamed (and done better)

clason commented

my only concern with the way things are for vim.system right now is I don't think it's as performant for doing line based items (which a lot of operations are for neovim). One thing that would be really awesome for vim.system would be some on_line or lines parameter or similar, and maybe some string wrangling in C to make things really zoom. But not sure, I'd have to do some benchmarks again.

That's not a bad idea; we have vim.iter now, and being able to feed stdout into that would be quite neat.

(I've also had situations where I'd have preferred a systemlist-type table output, but on synchronous jobs that's just a vim.split away, of course.)

lewis6991 commented

my only concern with the way things are for vim.system right now is I don't think it's as performant for doing line based items (which a lot of operations are for neovim). One thing that would be really awesome for vim.system would be some on_line or lines parameter or similar, and maybe some string wrangling in C to make things really zoom. But not sure, I'd have to do some benchmarks again.

vim.system() is just a light wrapper on uv.spawn() with pipe management. It allows stdout to be provided as a basic handler which a line based handler could be written on top of. A line based handler doesn't necessarily need to be built in to vim.system directly.

tjdevries commented

@lewis6991 from what I could tell, it always tries to put all the stdout into one variable at the end of execution? I have to go look again.

My main concern is when you do something like rg --files from home directory, i don't actually want to try and concat the string and save that all in memory (duplicated) again.

I can take a look again later tho.

lewis6991 commented

It only does that if you don't provide your own handler. If you provide a handler of the form:

function stdout_handler(err, data)
end

That will be used instead of collecting the data into a variable.

The main point of vim.system() is just to provide automatic handling of the luv objects.

tjdevries commented

ah, nice. thanks

clason commented

Would have really liked popup to have been fully fleshed out and made to match some of the vim apis, since that's a nice two-for-one kind of situation where a bunch of plugins can support vim and nvim with the same functions. But that doesn't look like it's going to happen (haven't seen any progress on that and I don't have any desire to drive those kinds of projects at the moment)

I think that project has become a victim of Neovim's success: before the post-0.5 Cambrian Explosion of Lua plugins (and Bram's vim9script push), enticing Vimscript plugin devs with such an API seemed like a great way of growing the Neovim ecosystem. Now I could count the number of plugins profiting from this on one (saw-mill worker's) hand since they'd have to

  1. be originally written for Vim only;
  2. make heavy use of popup UI;
  3. are established and don't have any Lua peers;
  4. don't have full Neovim-specific alternate code paths for this already.

As it stands, it would make much more sense to add a Neovim-specific high(er) level vim.ui.popup() API in core (designed fresh with all the lessons learned).

tjdevries commented

Yeah, but it does make situations like frameworks (denops comes to mind) where you want to have some shared API that works well in both quite nice.

(I just wanted to implement the vimscript APIs in Lua basically as wrappers around our code. A lot of it is quite nice for simple stuff that people generally do).

Yeah, it's probably not a huge win tho

juanandresgs commented

Updated plugins today and got the following error
image
Guessing it's related to this discussion on frencency/plenary?

NVIM v0.9.2 (LunarVim 1.3)
Build type: Release
LuaJIT 2.1.0-beta3
clason commented

Nope.

delphinus commented

@juanandresgs This is a bug by telescope-frecency.nvim. I fixed. Check it.

nvim-telescope/telescope-frecency.nvim#147

mrcjkb commented
  • plenary.busted / plenary.test_harness

With nvim -l it's quite easy to use busted with Neovim as the interpreter, without plenary. See my blog post on the topic for details.

Some examples where this has been done:

  • 🚀1