Add file preview

[tzachar]

Hi.

This is an initial attempt of adding file previews.
wdyt?

[hrsh7th]

I think it should be moved to source:resolve().

[tzachar]

Can I add documentation there?

[hrsh7th]

You should add resolve method for source.

source.resolve = function(completion_item, callback)
  -- read file and add documentation here.
end

[tzachar]

just did.

things to consider:

1. configuring the number of lines to show
2. not showing binary data -- maybe strip any non alphanum + space from the string?

[hrsh7th]

nvim-cmp will pass custom option like https://github.com/hrsh7th/cmp-buffer/blob/main/lua/cmp_buffer/init.lua#L28

{ name = 'buffer', opts = { keyword_pattern = [[\k\+]] } }

[tzachar]

nvim-cmp will pass custom option like https://github.com/hrsh7th/cmp-buffer/blob/main/lua/cmp_buffer/init.lua#L28

{ name = 'buffer', opts = { keyword_pattern = [[\k\+]] } }

but how to access that in resolve?

[hrsh7th]

You can pass the option to data field.

[hrsh7th]

Hm... Probably, we should pass option to the resolve/execute methods.

[tzachar]

well, copying the options through the data seems hackish...
are you willing to add the options to resolve?

[hrsh7th]

Hmm. resolve and execute only accept items and functions according to the LSP specification.
I can add options here, but it doesn't look beautiful. (I know it's convenient.)
I'm worried.

(google translated)

[tzachar]

well, maybe add a parameter to the source:new function?

[hrsh7th]

source.new = function()
  local self = setmetatable({}, source);
  self.option = { ... default ... }
  return self
end

source.complete = function(self, request, callback)
  self.option = request.option
  ...
end

I think it will be work.

[tzachar]

Yeah, but its still hackish... I though about adding an argument like this:

source.new = function(options)
  local self = setmetatable({}, source);
  self.options = vim.tbl_deep_extend('keep', option, defaults)
  return self
end

and then use self.options inside resolve.

[hrsh7th]

I think it is impossible.

Hm... we should introduce source.on_configuration_changed ? (it is similar to LSP's workspace/didChangeConfiguration

[tzachar]

Interesting.

But you should have good semantics regarding when on_configuration_changed is called.

[hrsh7th]

Yes. I think it is very hard to define semantics because sometimes the source is loaded lazily...

[tzachar]

Well, I can add a specific source config interface, but I think this should be solved in a more general fashion.

[tzachar]

anything new on this?

[hrsh7th]

I'm sorry. I forgot to confirm this PR.
I've review it now. Please check it.

[tzachar]

I think I addressed all the issues.

[hrsh7th]

I think we should add binary file detection or disable preview option.
What do you think about it?

[tzachar]

How would you detect binary files?
And I am all for adding an option for disabling preview, but the question remains how to support per source configuration. Would you like to add some infrastructure in cmp, or do you want me to add a setup function specific to this source?

[dmitmel]

You could do what Git does (IIRC), check if the first kilobyte includes a NUL byte. (response to "How would you detect binary files?")

[hrsh7th]

My vimscript version implementation is here.

function! vimrc#is_binary(filepath) abort
  return stridx(get(readfile(a:filepath, 'b', 1), 0, ''), "\<NL>") != -1
endfunction

(I don't know how to implement it in Lua)

[tzachar]

I added just this functionality in f7d350a

[dmitmel]

@tzachar Try doing bfile:read(1024):find('\0'), it will be way more efficient

[tzachar]

@hrsh7th should I add setup functionality to enable this feature?

[hrsh7th]

I'm considering configuration API.

local source = {}

source.on_configuration = function(self, params)
  self.... = params....
end

...

return source

[hrsh7th]

1. Ship it with always enabled preview for now
2. Ship with configuration ({ name = 'path', opts = { preview = true } }) and hacky codes.

[tzachar]

Cool. Do you want to wait till u stabilize it?

[tzachar]

@hrsh7th not sure I understand. Do you want me to add anything or will you merge it as is?

[tzachar]

@hrsh7th Anything new on this?

[tzachar]

@hrsh7th
This is frustrating.
If you do not intend to merge this, please close the pull request.

[hrsh7th]

I'm sorry to forget about this PR every time...

[tzachar]

10x