telescope.nvim icon indicating copy to clipboard operation
telescope.nvim copied to clipboard

feat(previewer): clean up hooks & lever open_term

Open fdschmidt93 opened this issue 3 years ago • 5 comments
trafficstars

As a very first crack

  • Cleans up the hooks (originally my bad :sweat_smile: ) which would be a breaking change (not saying this is ideal, but I feel something along these lines should be done before 0.1, just as an initial proposal) (TODO: add deprecation notice)
  • Lever nvim_open_term

The term previewer can be used with something like this

require "telescope".setup {
  preview = {
    command = function(filepath, bufnr)
      -- maybe we should try to avoid having to schedule here
      vim.schedule_wrap(require("telescope.previewers.utils").buf_term_preview)(bufnr, {
        command = "cat",
        args = { filepath },
      })
      -- but a bit funny since user should indicate file has been previewed
      return true
    end,
  },
}

TODO

  • [x] Proper scroll handling (above previewer doesn't adjust scrolling for eg live_grep)
  • [ ] Let's see if we can clean up previewing a bit more
  • [ ] I guess use nvim_open_term for previewers where it makes sense

Preview of what will be relatively easily possible ;)

image

fdschmidt93 avatar May 02 '22 20:05 fdschmidt93

The hooks cleanup looks good. I am not sure about preview.command thought. I feels like another interface in a "sea of interfaces". The same should be achievable with (not tested)

require "telescope".setup {
  default = {
    buffer_previewer_maker = function(filepath, bufnr, opts)
      require("telescope.previewers.utils").buf_term_preview(bufnr, {
        "bat", filepath,
      })
    end,
  },
}

Thats something i wanna address for post 0.1. Have one interface for something where you can say either use our predefined function or write your own.

You could argue doing that above will lose the hooks functionality, etc. But you dont need mime type anyway because bat does that for you (i hardly think people would use cat here). I am not sure if file size has an influence and is needed.

Edit: Another thing i've been thinking is that previewers.new_termopen_previewer exists for previewing shell commands. Maybe we should update that with nvim_open_term with the same interface on top just on top of new_buffer_previewer. And than we should move the git_/man previewer back to termopen (because that was a mistake that i made anyway)

Conni2461 avatar May 03 '22 06:05 Conni2461

I feels like another interface in a "sea of interfaces"

Agree, let's revisit this.

You could argue doing that above will lose the hooks functionality, etc.

Yeah, that's more for the vim buffer file previewer. Or something like file_browser where you might want to have a hook for folders to show tree or something.

Thats something i wanna address for post 0.1. Have one interface for something where you can say either use our predefined function or write your own.

I mean it shouldn't be too difficult to clean this up in one sweep hopefully, I feel like this should be one of the "breaking changes" to mark 0.1 maybe? But maybe that's bad practice. Not sure. I'll just try to progress here in the meantime.

And than we should move the git_/man previewer back to termopen (because that was a mistake that i made anyway)

Sorry, the edit reads confusing -- you are saying that ultimately these previewers should be moved to nvim_open_term ones? :sweat_smile:

fdschmidt93 avatar May 03 '22 07:05 fdschmidt93

you are saying that ultimately these previewers should be moved to nvim_open_term ones

Maybe, just saying that this could be a better approach than a new interface.

"breaking changes" to mark 0.1 maybe

I am not planing to do that much more breaking changes for 0.1. We should find a date for a get together :grimacing:

Conni2461 avatar May 03 '22 07:05 Conni2461

For posterity:

nvim_open_term is funny. The last lines of the buffer are typically empty (not sure why, probably something with rendering and/or input stream).

Setting to cursor is solved as follows. nvim_chan_send entire command output and then check that last non-empty line of command output and preview buffer are equal up until at most timeout. The command output requires escaping ansi codes. I verified the check passes within lua files of telescope frequently. (I happened to write a couple of 100MB telescope logs until I followed grokked what's going on :sweat_smile:). Once last non-empty lines are equal, we can safely proceed to set cursor to top of buffer.

Lastly, due to async nature of nvim_chan_send and job execution, we require a lot of safeguards to make sure we don't get unwanted errors if users smash buttons.

fdschmidt93 avatar May 05 '22 19:05 fdschmidt93

wrt chore: remove schedule_wrap from fs_stat and clean up hooks

If I'm not mistaken just wrapping the async callback of fs_stat allows us to get rid of most scheduling within the callback itself.

fdschmidt93 avatar May 05 '22 20:05 fdschmidt93