impromptu.nvim

impromptu.nvim copied to clipboard

Hey,

I wanted to try your new shiny impromptu.filter, but I can't get it to work. I've tried this:

  impromptu.filter{
    header = "HEADER",
    lines = { 
      { description = "Line 1" },
      { description = "Line 2" },
    },
    handler = function(b, opt)
      print("option: "..tostring(opt))
      return true
    end,
  }     

but I'm getting an error about line 150 of filter.lua getting a nil argument to ipairs. Did I miss something?

Apparently I mistyped the argument name in the docs. It should be options instead of lines.

Ah yeah, the docs were somewhat unclear on that, it says lines here, but options here, but since the latter also has too many braces, I figured that would be wrong without really thinking about that :)

Other than that, seems to be working. You did not document the keys, but I found <C-k> and <C-j> somewhat surprising, and would personally prefer <C-n> and <C-p> in accord to the usual vim usage.

Is the header used anywhere btw? I can't see it anywhere :)

Two more observations:

1. I wasn't bitten by this, but lua's find takes a pattern, which might be surprising. Try e.g. filtering by %. I suggest passing plain = true to find as a default.

2. The dots as a separator look pretty ugly, at least in my setup, due to spacing reasons (they are drawn in the middle of the cell, which make the space above the typed characters larger then below them). I suggest simply changing it to an underscore.

Bonus: Can I get an empty top line here?

Ok, let me try to address all of this:

• maybe I completely messed up the documentation, it's title, not header;

• it makes sense mappinf to <C-n> and <C-p>. I'll just add that I intend to provide means for the user to add custom mappings.

• nice catch about the find option. I'll add that.

• The dots look nicer on my setup, but I understand that's debatable. I'll add ways to change it and default to dashes (as it's done for the top split)

As for layout stuff, I put up this screenshot so you can judge for yourself. Not a biggie, of course :)

From the things that you mentioned, still missing for further commits are:

• [ ] Fix the documentation
• [ ] Remap keys
  • [ ] Allow custom remapping
• [ ] Change div/subdiv to use from config
  • [ ] Expose config as argument
  • [ ] Set global config

Hey, I've tried the latest version, but I don't see how I need to change my code. I get an error from this line because iter is a table value. My code would be here. Thanks!

the filter_fn should be an iterator, not a function that returns the iterated elements. That being said, it shouldn't be swapped unless it really makes sense to. I'd rather make the default behaviour to be case-insensitive that have that as a reason to swap the filter_fn.

The reason for it to be an iterator rather than returning the whole list is that the list of potential matches could be gigantic and iterating over all of them to get the result can be overkill. There's another iterator that ensures we take only the amount of results we want to see.

That allows us to save needless computation (by only filtering when asked to and by only taking the amount of items we want to).

I'm adding a commit that makes it case insensitive.

Cheers

Makes sense, good call! Not sure if case-insensitivity is the proper thing in general, but it felt totally right in my case ;)

I believe case insensitive is good in general, but maybe I should leave it to be configurable.

Also, I'll eventually try to make search a fuzzy finder instead.

I've now removed filter_fn from my filter call, but I get an error because on line 205 of filter.lua, hls is a nil value.

Sorry for all those detailed questions, but I can't figure it out from the code or the docs. Are the latter up to date w.r.t. the names of the arguments, by the way? I think filter takes title and options rather than header and lines, doesn't it?

I think you have an outdated version. The hls error was solved by #37.

The docs are still outdated, I thought I had solved it but I left it out it seems.

Ahh darn, sorry, I botched my git pull. The error has gone away, but it's behaving strangely: Typing does not filter the menu (but the test appears in the buffer). Did something change I did not notice? The code is still this one, but with filter_fn commented out.

One problem btw might be that the buffer is modifiable. Probably needed for the filtering, but thought I'd mention, since I've already managed to botch stuff :D

Can you share a print of the issue? I'll investigate further your code to see how come this is happening...

What's a "print"? Here is a screenshot, if that helps.

Sorry, that's what I meant (from PrintScreen).

It's weird that it also didn't highlight the match.

@KillTheMule could you try my latest commit (acdd34d)? Locally everything is running fine here so I suspect there might be something with your setup. I'll try your code later on.

I've switched to that branch with that commit, but it's all the same. I've also noticed pressing "Enter" puts in a newline, rather than selecting the entry. I'm using -u NORC btw.

Which neovim version are you using? It seems none of the mappings and autocmds are running..

I assume if you press <C-c> or <C-j> nothing will happen.

Could you paste me here what's the output of a :map <buffer>?

I'm on a pretty recent HEAD version. :map <buffer> shows "no mapping found". The keypresses do whatever the usually do in insert mode.

Ok, that gave it away, it's this line. Changing ~= to == fixes it.

Check against master now. Everything should be working fine.

Ok, filter seems fine now, gonna try the config options now :)

Should I keep asking here, or make new issues? There was a change in ask, namely the 2nd arg of the handler function. Before, it was the option_key itself, now it's the corresponding value. Is that intentional? I can adjust, of course, but figured I'd ask first :) The advantage of getting the key is that one can get the value easily (since one passes the opts array, one has access to it, so it's a simple lookup), which retrieving the key from the value is more complicated. No problem, just let me know what your intention here is, please :)

(e) Have you already implemented using the config? It looks like your correctly taking the config when passed, and default otherwise, but you're not using it anywhere, are you?

(ee) Another tiny thing: Impromptu relies on the highlight Groups Comment and Keyword existing. This should almost always be the case indeed, but might not under some circumstances, namely a missing runtime (I found out when my functional tests broke, because the neovim test runner does not load the runtime files). A nonintrusive way to define those groups always is to run hi def Comment NONE.

Ok, I ended up committing the change in ask. It is intentional.

Forms, prompts and filters can be persistent. This is why you have to return true to close the window.

Passing the table as argument, specially now that ask can do highlight, allows you to visually indicate that such option is toggled if desired.

Also, the api gets slightly closer to form and filter.

Finally, it is difficult to get the leaf object from the tree if I give you only the key to it.

You can keep this open as long as you feel there are improvements to be done on filter.

And for the config, I'm still implementing it. The idea is to have a global config that can be locally overridden by plugins, so behavior is consistent for every plugin that uses it.

Also, users could change the config themselves and that'd be global to impromptu I'd say. This is what I have in mind for config, but it still isn't implement more than the config api itself.

Thanks for the tip with the highlights. I'll make sure I have them correctly by actually using something else that I define and control. That also could be subject to user/plugin configuration.

Thanks for the clarifications! An yeah, using your own HL groups is probably the most sensible thing anyways.

Ah, another question if you don't mind. What are your assumptions on the keys of the options in ask's options? Seeing that the value is passed to the handler instead of the key, I tried using integer keys, but that failed. Do they need to be strings?

I've built ask with the assumption keys would be strings and never tested with integers. Maybe that's the case.

Aaannd an improvement idea: Using <C-j> selects the next option in filter, but you can't scroll if the number of shown options is larger than the buffer size. That's a bit of a restriction, e.g. imagint this menu where you want to insert a Material card, i.e. you type Mater... and then you have a long list, you can't see all of the entries, but you can't filter further because all those numbers are somewhat arbitrary. I can imagine that this is something that could come up for other people as well.

(Let me stress again that those are ideas, my menues won't really see any real use, so I personally don't mind much)