Save position into jumplist before 'edit' action.

[caojoshua]

fixes #1038
fixes #1520

Whenever telescope does not sort of edit/selection action, it calls nvim_win_set_cursor(), which does not add anything to the jumplist. As noted in the issue, this affects lsp_references picker. This PR adds the current location to the jumplist, as neovim builtin LSP does when jumping to location.

Builtin LSP also adds the current word to the tagstack. I didn't add it in this PR because I don't want to copy and paste code, but if we want it I can quickly add it in.

This change affects all edit/selection actions, and I'm not sure if its desirable in all cases and pickers. Open to discussion.

[Conni2461]

I am not sure if we want to do it for every picker. Give me a day to think about it and maybe test it (i actually dont know if thats an issue for find_files, grep_string, etc)

[caojoshua]

I get you, I'm not sure if I would want to do it for every picker either. But it would also be tedious to leave a mark for some pickers and not others. Not sure if there is a cleaner way to fix the linked issue.

[Massolari]

I don't know if it makes sense for every picker, but I came across this issue because I want this for document_symbols(), so, limit this only for references() maybe won't be good enough

[caojoshua]

Seems people have started to care about this, so I started thinking about it again. Here I added a picker option push_cursor_on_edit (very open to better naming) to add a jumplist entry when using an edit action. With this, you could do:

lua require('telescope.builtin').lsp_document_symbols({ push_cursor_on_edit = true})

I don't provide push_cursor_on_edit = true by default for any pickers, and leave it open to individual configs. Although if users feel strongly about particular pickers having the action on by default, I can make the change in this or another PR.

Other options I considered:

• Separate actions for edit_and_push_cursor. Users can override actions mappings as needed. Disadvantage is that separate mappings are required for each action to edit regular, tab, split, and vspit.
• Push cursor when instantiating the picker. Don't think users want this though, because it will also push to jump list when user exits out of telescope without an edit action.
• Support callbacks on closing Telescope window. Seems unnecessary as I can't think of other use cases for it.

[Conni2461]

I dont like having a config option. We can make it only be a opts and we set this value manually for the pickers that require it. What do you think?

Support callbacks on closing Telescope window. Seems unnecessary as I can't think of other use cases for it.

I want this regardless how this ends. For basically everything inside telescope. OnPromptChange, OnLoaded, OnClose, .... But this is a post release feature, we are still planing, on how we wanna realize it.

[caojoshua]

If we're going to make it opts, then why not also make it config? Users can override opts, so if we leave it out of default config, wouldn't just be an undocumented config without a default?

[Conni2461]

Yes thats the point. Its can be an undocumented internal thing until we introduce some real event system where we && people can hook stuff into. Because i dont like the solution but we should still fix this issue. I think it doesnt make sense to configure that on a global level but should rather be configured by us here for example: https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/builtin/lsp.lua#L79

[caojoshua]

Only change is I removed push_cursor_on_edit from default config.

but should rather be configured by us here for example: https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/builtin/lsp.lua#L79

Is this something we want in this PR? This change would require a change to every picker that wants the new behavior, and I don't want to be the one making those choices.

[Conni2461]

Sorry for the late response. I added the new option to the mentioned builtins and tested it.

Thanks for the PR and your patients :)