Cycle prompt history

[Conni2461]

Close #520

Showcase: https://streamable.com/w3wpri

Enabled on default you only need to add mappings

require('telescope').setup {
  defaults = {
    mappings = {
      i = {
        ["<C-Down>"] = require('telescope.actions').cycle_history_next,
        ["<C-Up>"] = require('telescope.actions').cycle_history_prev,
      },
    },
  }
}

For more information :help telescope.defaults.history

CC @saragiotto

[saragiotto]

Maybe I'm doing something wrong or it's related with the PR pointed out on the description.

Anyway, after the installation and configs setup, I'm facing the above error when trying to open files:

E5108: Error executing lua ...im/plugged/telescope.nvim/lua/telescope/actions/init.lua:44: attempt to call method 'write_text' (a nil value)

@Conni2461 did you've any clue on that?

[Conni2461]

Oh yeah i have an update for that that i didn't push because i wasn't done yet 😆 I will just push the fix for that now :)

Edit: I have some issues with not correctly resetting the index on exit and more stuff. I try to work some more on it later :)

[Conni2461]

Another new version that should work better now. History gets loaded only once on first time. I still need to clean that up in a separate module and add a history_limit.

[Shatur]

@Conni2461, what is the status of it?

[Conni2461]

I was super busy for the past few weeks but this will change next Wednesday. Then i will finish it. Shouldn't take that long :)

[Conni2461]

Okay this is now somewhat done. I changed the name of the config var to history_location and added history_limit (for full example see top comment, its updated).

Short:

history_location = '~/.local/share/nvim/telescope_history', -- default nil, history disabled
history_limit = 1000, -- default nil (limiter disabled),

[kkharji]

@Conni2461 awesome pr, it's really an important missing feature, @saragiotto great suggestion

#520 (comment) It keeps in just one big file, mine gets 1000 lines, I think that is the maximum. But definitely It's not based on cwd/project.

I kinda disagree, I mean I can understand why it shouldn't be unique or should be unified for your use-case as I presume that you intend to use it only for the very last input in case you accidentally closed the picker, or when you open several files in order and want to easily switch between them. But what about other use-cases? for example

• Imagine you opened a command history or highlight or builtins picker, would like to see unrelated input history?

• You are work on multiple project, in multiple tabs, one of them is rust and the other is python, would like prev to show say; cargo.rs instead of setup.py?

Thus, I strongly dismiss the idea of a unified history for ALL pickers, One of the core features of telescope is that every Picker has it's own world (preview, theme, layout, mappings .. etc) and history shouldn't be treated differently. History, I believe, should be unique PER picker and when the picker is a variant of file/grep finder it should include PRE cwd or search_dirs. Perhaps pickers such as buffer picker might be better skipped and only saved in local variable since it's unique to the vim instance.

[Conni2461]

Idea:

1. A simple, unique history here (as it is right now) with the ability to override it (similar to sorters)
2. A complex context sensitive history as extension utilizing sql.nvim. Metadata is picker, cwd (for now) because it would be kinda annoying to do this in plain text and maybe \t separators.
   And with that extension idea i can make everyone happy :)

Thoughts @Shatur95 @saragiotto @elianiva @tami5

I would start working on this Saturday. I wanna get this done as soon as possible. :)

[elianiva]

Seems like a good idea to me, though I'm not sure what "context sensitive" history is 😆

[Conni2461]

Basically what tami said. A history for each picker (i just thought context sensitive made sense, maybe not).
So a history for find_files + cwd and another one for grep_string + that_cwd. and another one for grep_string + other_cwd, etc

[elianiva]

Right, that would be nice.

[sandangel]

hi, may I ask for an update on this PR?

[Conni2461]

I just rebased it. Sadly the action api is lacking, so history will not be saved in all pickers. Missing are the once that do actions.select_*:replace. Examples are the git once and man_pages/help_tags (and probably more, looking at extensions). I mentioned this here: #521 (comment) and tj wanted to fix the action API here #807 but he hasn't gotten around to finish it.

I could implement a workaround for that but i haven't asked tj if that is the best idea. I will do that when he comes back from his vacation.

But there is no reason why you couldn't check out the branch and run it. :) I would appreciate some feedback ;)

[sethidden]

I feel as if actions.cycle_history_prev/next should throw an error to :messages if the user hasn't set history_location. If you bind the actions (C-p, C-n) but forget to set history_location, pressing the bound keys does nothing (which is obviously expected behavior)

[RRethy]

I feel as if actions.cycle_history_prev/next should throw an error to :messages if the user hasn't set history_location. If you bind the actions (C-p, C-n) but forget to set history_location, pressing the bound keys does nothing (which is obviously expected behavior)

Why not default to somewhere in $XDG_DATA_HOME? The less required configuration the better.

[Shatur]

Why not default to somewhere in $XDG_DATA_HOME

There is vim.fn.stdpath. I would use data dir for it.

[Conni2461]

AFAIK its not enabled on default for fzf. Enabling it on default also requires mapping cycle_history_prev/next and this is a nightmare because we can't break backwards compatibility, <C-p> or <C-n> would. Its a way to opinionated topic that i don't want to make any decision here. We currently already have mappings that are not ideal in insert mode (<C-u>) and we should learn for that "mistake".

I checked and it seems that i am currently suggesting <C-Up> <C-Down> in the documentation which is (IMO) also not ideal but i think tj suggested them.

But i don't see a reason to print a error message if history isn't enabled and the actions are mapped and/or used