Simplify neovim servernames for inverse search

[lervag]

As mentioned in the (looong) discussion in #2219, I wanted to test a simple method for managing the neovim servernames. This PR implements it. My first impression is that this works well, but I would be happy to get some feedback.

@clason @PanagiotisS

[PanagiotisS]

What can I say. It works. It's fast on my SSD (which is a concern of mine when using files).
I tested by

1. opening a file in nvim (terminal),
2. then opening a second file in nvim-qt,
3. closing nvim

It worked on every step!

Nothing to add

P.S.

A: Yes, but some people may complain that VimTeX is not written in Lua!

I had a good laugh when I read it :)

[clason]

Yes, that works nicely, even when I start Skim through Finder!

A few thoughts:

1. I get duplicate entries in the nvim_servername.log file for multi-file projects (i.e., every time I jump to a new subfile in the project the name gets added again).
2. Is the cache ever pruned? I assume there is very little chance of collision (later server names happen to be the same as an earlier one), but the list could get very long indeed otherwise... (Especially if the names are random, so every time I reopen the same file I get a new name.)
3. I think the documentation for :VimtexInverseSearch could be improved: right now, it unconditionally suggests using nvim --headless -c "VimtexInverseSearch %l '%f'", but that is wrong for Skim (where it needs to be %line and %file). In the vimtex-view-skim section, this is correctly explained, but if someone only looks at the first section, they could be mislead. Maybe add a sentence about checking the viewer-specific section to get the correct options?
4. You know you can use Lua from vimscript, right? ;) (Using lua << EOF ... EOF heredoc syntax.) So you could in principle swap out single functions or even single commands (where it would give a benefit, of course) without having to rewrite the whole plugin.

[lervag]

It worked on every step!

Nothing to add

Great, thanks!

A: Yes, but some people may complain that VimTeX is not written in Lua!

I had a good laugh when I read it :)

Haha, I was hoping for some reaction, but I didn't expect it so fast :D

[lervag]

Yes, that works nicely, even when I start Skim through Finder!

Great, good to hear!

I get duplicate entries in the nvim_servername.log file for multi-file projects (i.e., every time I jump to a new subfile in the project the name gets added again).

Ah, good point; I'll run the list through a unique filter, then.

Is the cache ever pruned? I assume there is very little chance of collision (later server names happen to be the same as an earlier one), but the list could get very long indeed otherwise... (Especially if the names are random, so every time I reopen the same file I get a new name.)

Yes and no; the cache is pruned on every invocation in the sense that any server that we can't connect to will be removed from the list:

vimtex/autoload/vimtex/view.vim

Lines 144 to 148 in 4bd7e32

	try
	let l:socket = sockconnect('pipe', l:server, {'rpc': 1})
	catch
	continue
	endtry

I think the documentation for :VimtexInverseSearch could be improved: right now, it unconditionally suggests using nvim --headless -c "VimtexInverseSearch %l '%f'", but that is wrong for Skim (where it needs to be %line and %file). In the vimtex-view-skim section, this is correctly explained, but if someone only looks at the first section, they could be mislead. Maybe add a sentence about checking the viewer-specific section to get the correct options?

Good point!

You know you can use Lua from vimscript, right? ;) (Using lua << EOF ... EOF heredoc syntax.) So you could in principle swap out single functions or even single commands (where it would give a benefit, of course) without having to rewrite the whole plugin.

Yes, I'm aware, and it's a good point. Although I don't know where I would currently benefit from this possibility.

It's not impossible that I at some point in time go for a "neovim only" future. In this case, it makes sense to rewrite certain parts in Lua for efficiency. But as long as I do Vim + neovim support, I want to keep things on Vimscript if possible to avoid duplicate code.

[lervag]

@clason I notice I already did add a comment on %l and %f, here:

vimtex/doc/vimtex.txt

Lines 5231 to 5233 in 4bd7e32

	file are provided as interpolation variables. In the following, we use `%l`
	and `%f`, but the interpolation variables may be named different in some
	viewers (e.g. |vimtex-view-skim|).

Do you think we should make it even more clear?

[clason]

Ah, good point; I'll run the list through a unique filter, then.

Yes, that's a good idea!

Yes and no; the cache is pruned on every invocation in the sense that any server that we can't connect to will be removed from the list:

Ah, ok, that's good then (I was thinking how to avoid "pulling the rug" out under other running instances... (An alternative would be to do it on startup, but I'm not sure that'd be much better.)

Do you think we should make it even more clear?

Ah, I looked at help :VimtexInverseSearch and didn't scroll back up. My suggestion would be to combine the two sections (they're rather short, and can probably be condensed further?) so readers are guaranteed to hit that (good) remark?

Yes, I'm aware, and it's a good point. Although I don't know where I would currently benefit from this possibility.

Nothing currently comes to my mind, either; I just wanted to mention it for the sake of completeness (as it was brought up).

It's not impossible that I at some point in time go for a "neovim only" future. In this case, it makes sense to rewrite certain parts in Lua for efficiency. But as long as I do Vim + neovim support, I want to keep things on Vimscript if possible to avoid duplicate code.

Ah, I was only talking about the parts that are already "neovim only" (as alternative code paths), not suggesting to add more alternative or optional things for the sake of writing them in Lua. I'm sure it's possible to write a great NvimTeX, but that would be a different plugin!

[lervag]

Yes and no; the cache is pruned on every invocation in the sense that any server that we can't connect to will be removed from the list:

Ah, ok, that's good then (I was thinking how to avoid "pulling the rug" out under other running instances... (An alternative would be to do it on startup, but I'm not sure that'd be much better.)

I'm not so worried about "pulling the rug". It would make sense to check the servers on VimTeX startup, but unless there should be evidence that the current version fails somehow I'll leave it as I think things work well now.

Do you think we should make it even more clear?

Ah, I looked at help :VimtexInverseSearch and didn't scroll back up. My suggestion would be to combine the two sections (they're rather short, and can probably be condensed further?) so readers are guaranteed to hit that (good) remark?

Ok; I was thinking of it as a single section, except I tagged the command to where it was first described. I'll look at it again and see if I can improve further.

I'm sure it's possible to write a great NvimTeX, but that would be a different plugin!

Agreed; and for another time.

[clason]

I'm not so worried about "pulling the rug". It would make sense to check the servers on VimTeX startup, but unless there should be evidence that the current version fails somehow I'll leave it as I think things work well now.

By "pulling the rug", I mean starting one nvim session, then starting another one for a different project, then closing the first one -- which shouldn't wipe out the server name for the second session. (Which is not the case in your implementation but in the naive ones I came up with...)

So, yes, no reason to change what you have now.

[lervag]

Thanks!

I believe this is ready for merge, and unless anyone suggests otherwise I'll merge tomorrow.

(I also believe it is about time for another release!)

[PanagiotisS]

After reading the conversation, a concern might be what if the user never runs InverseSearch?
If the closed servers are removed from the log file only after running InverseSearch, but the log file is populated each time a new .tex file is opened, then soon the log file would become big.

P.S. Since this part is neovim only it might be your opportunity to write some lua code! :)

[clason]

After reading the conversation, a concern might be what if the user never runs InverseSearch?

That's where my concerns were going as well. Maybe it would indeed be better to prune the cache on startup (or quit)?

(This might also be more parsimonious than checking all servers on every inverse search?)

[lervag]

After reading the conversation, a concern might be what if the user never runs InverseSearch? If the closed servers are removed from the log file only after running InverseSearch, but the log file is populated each time a new .tex file is opened, then soon the log file would become big.

I can see your point. I investigated if moving the cache file pruning would impact startup time, and it seems to be negligable (<~ 1ms). So I've updated accordingly.

(This might also be more parsimonious than checking all servers on every inverse search?)

Parsimonious? ... new word for me, but after checking the dictionary; I agree. It moves the penalty to VimTeX initialization. But I think this penalty is realy not noticable in any case.