Implement better inverse search #2219

lervag · 2021-10-25T08:53:59Z

This PR is inspired by #2217. The idea is to implement the inverse search functionality within Vimscript for both Vim and neovim, so that the viewer configuration can be simplified. I.e., when this is finished, a viewer can run something like this:

# For Vim
vim -T dumb --not-a-term -n -c "call vimtex#view#inverse_search_comm(%l, '%f')"

# For neovim
nvim --headless -c "call vimtex#view#inverse_search_comm(%l, '%f')"

Then the function inverse_search_comm will pass this on to all available Vim/neovim instances. The idea is that each instance will inspect the received arguments and only perform the inverse search if relevant.

Pseudocode:

vimtex#view#inverse_search_comm:

for server in serverlist
  pass_to_server: vimtex#view#inverse_search(line, file)

quit

and

vimtex#view#inverse_search:

if (not in vimtex buffer) or (not in correct vimtex project): return

navigate to: line and file

Remaining tasks:

lervag · 2021-10-25T09:15:36Z

I'll work more on this as I think there are more "rough" edges yet. But I think it should already, possibly, work. So feel free to test!

This was your idea, @user202729, so I hope you can help with testing. Which system are you on and which version of Vim?

@clason I'd also appreciate some tests from your side with Skim and MacOS. You could already test, or you could wait until I've refined this some more.

Btw: When this works, I can actually remove the dependency on neovim-remote(!), and we can simplify the docs quite a bit as this should, in principle, work in all cases, on all systems. (Famous last words, and everything!). (So, @user202729, it seems I am starting to think you had a great idea, here...)

clason · 2021-10-25T10:18:02Z

Hmm, that doesn't seem to work for me. If I run the command from the shell, I get

Error detected while processing command line:
E492: Not an editor command: vimtex#view#inverse_search_comm(100, 'regularity.tex')
Press ENTER or type command to continue

Should this be call vimtex#...?

clason · 2021-10-25T10:29:35Z

(I'll think about how to do this with libuv -- would be nice to get rid of the pynvim dependency, too!)

lervag · 2021-10-25T10:33:11Z

Sorry, it should be -c "call ..." (see updated post).

I don't think there is a pynvim dependency here, but there is a python dependency in that I use psutil to get the list of servers. I would be happy to do that in a better way!

clason · 2021-10-25T10:44:28Z

Sorry, it should be -c "call ..." (see updated post).

Hmm, that just "freezes"... The Python code works, though.

I don't think there is a pynvim dependency here, but there is a python dependency in that I use psutil to get the list of servers. I would be happy to do that in a better way!

You're right, you don't need to access Neovim from Python. Then it's less critical (although zero dependency would be even nicer ;))

user202729 · 2021-10-25T11:17:18Z

I didn't notice that VimTeX passes the servername to zathura. In that case isn't this feature (and the complex configuration in the documentation) completely unnecessary? (somehow I didn't notice that.)

Actually it's still useful when zathura is started separately (need zathurarc configuration), but for the "zathura called from VimTeX" instance you don't need to loop (but another loop isn't a problem if it simplifies the code.

Future note: there should be some note in documentation regarding VimTeX passing custom -x (actually it already says "pass the servername"), otherwise you risk the user being extremely confused about why their zathurarc configuration does not work.

(unrelated: previously I was also extremely confused about why passing -xelatex to vimtex_compiler_latexmk does not work, because VimTeX overrides it, but with the change in TeX directive parsing it's fine now)
UNIX allowing arbitrary characters in the file name is terrible for the developers. Although it looks like that the current implementation will only fail for ' (or newline?) in filename
Related to the issue above, when there's a space in servername the plugin does not work.
Should it warn the user if there's no server started, or should it
just automatically start one?

clason · 2021-10-25T11:36:32Z

Ah, it's probably not working because vimtex#view#receive is not implemented yet ;)

user202729 · 2021-10-25T12:16:06Z

Regarding my vim version:

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled May 28 2021 06:58:52)
Included patches: 1-2891
Compiled by Arch Linux
Huge version with GTK3 GUI.  Features included (+) or not (-):

TODO: see if \input/\include cause new problems.

I think the GUI one has the additional feature that spawn a new instance if there's no existing (would be nice to have, but require the user to specify the terminal command-line to use)

lervag · 2021-10-25T13:44:40Z

@user202729

I didn't notice that VimTeX passes the servername to zathura. In that case isn't this feature (and the complex configuration in the documentation) completely unnecessary? (somehow I didn't notice that.)

Yes, but only if you use Zathura. For every other viewer, one does need some manual configuration.

Actually it's still useful when zathura is started separately (need zathurarc configuration)

Yes.

, but for the "zathura called from VimTeX" instance you don't need to loop (but another loop isn't a problem if it simplifies the code.

Agreed - I've not decided if we should "short circuit" in this case or not. There is a very short extra delay. Barely noticable, but it is there.

Future note: there should be some note in documentation regarding VimTeX passing custom -x (actually it already says "pass the servername"), otherwise you risk the user being extremely confused about why their zathurarc configuration does not work.

Agreed.

UNIX allowing arbitrary characters in the file name is terrible for the developers. Although it looks like that the current implementation will only fail for ' (or newline?) in filename

I partly agree; I find the restrictions in Windows to be equally terrible. In any case, I agree it is hard to be 100% robust here. It will fail on ' and possibly also other characters. Let's deal with these things only if necessary.

Related to the issue above, when there's a space in servername the plugin does not work.

If you find this to be important, then please open a new issue when the current PR is merged.

Should it warn the user if there's no server started, or should it just automatically start one?

I would not automatically start a server. And it is hard to issue a warning in this case. I think instead we should make the docs clear enough so that people configure and use Vim correctly. This should not be a problem on neovim, where there is always a server.

I think the GUI one has the additional feature that spawn a new instance if there's no existing (would be nice to have, but require the user to specify the terminal command-line to use)

Not sure what you are referring to with "the GUI one". But I'm not sure if I want to make the callback start a new instance of Vim automatically. I think instead people should setup things correctly on their side. We can lower the barrier, but we can't put it on the floor. :p

@clason

Ah, it's probably not working because vimtex#view#receive is not implemented yet ;)

Yes, sorry. I've pushed updates now that seem to work well for me.

lervag · 2021-10-25T13:45:10Z

I've added a remaining tasks on the top post.

clason · 2021-10-25T13:50:23Z

Hmm, sorry, still not working for me (on macOS). The invocation on the command line just "freezes", and nothing happens.

lervag · 2021-10-25T13:52:13Z

Yes, please close all neovim instances and restart them before testing. This feature will ask all neovim instances to run a function that may or may not be updated to the most recent version. I've failed to find a way to reliably force it to timeout. But I believe this is only a problem now while developing - when it works as it should, there should be no such issues afterwards.

user202729 · 2021-10-25T13:56:00Z

For the GUI part, I mean that there's a feature like this (not sure if it's intentional, and it's trivially workaround-able anyway)

starts the viewer independent from vim
"click" on the item in the viewer

while there's no gvim opened, it will open a gvim instance.

lervag · 2021-10-25T13:57:23Z

For the GUI part, I mean that there's a feature like this ...

Thanks, I understood that. My point was that I'm not really interested in supporting this part.

clason · 2021-10-25T13:57:34Z

Oh, that goes without saying. The issue is that Skim doesn't like that command -- every inverse search leaves a zombie nvim process around (and it doesn't search even the first time).

It does work from the command line if no zombie instances are running.

lervag · 2021-10-25T13:58:46Z

I've changed from rpcrequest to rpcnotify, which I think solves this problem.

clason · 2021-10-25T14:29:01Z

Ah, OK, I see what's happening. Skim uses %line and %file (instead of %l and %f) -- and %file is always the full path. So I still need to have Skim launched by VimTeX. But then it works (even with multiple instances).

autoload/vimtex/view/zathura.vim

lervag · 2021-10-25T16:00:29Z

Ah, OK, I see what's happening. Skim uses %line and %file (instead of %l and %f) -- and %file is always the full path. So I still need to have Skim launched by VimTeX. But then it works (even with multiple instances).

Yes, that seems right. Although, I would have thought things to work even if Skim is not launched by VimTeX. So that is actually unexpected...

clason · 2021-10-25T16:01:31Z

Indeed; I don't know why, yet... (Possibly -- and most likely -- an environment issue.)

EDIT Yep, it's enough to open -a Skim from the shell to make it work. Not sure exactly which environment variable(s) it needs, though...

lervag · 2021-10-25T18:16:50Z

Ok, I've pushed a big update to the docs now. It really is a significant simplification, so if this really does end up working as I hope it will, I believe it will be a very useful improvement!

lervag · 2021-10-25T20:47:59Z

Ok, I've tested with both Zathura and Okular on Linux now, with both Vim and neovim. Everything seems to work very well.

Now it remains to have this properly tested with:

Vim on MacOS (Skim)
neovim on MacOS (Skim)
Vim on Windows (SumatraPDF)
neovim on Windows (SumatraPDF)

I know you already tried a couple of times before, @clason, but now I'm quite sure that things should work in general. There might be a minor issue if you open Skim in a strange way, although I don't understand why. I would appreciate feedback here.

@jdhao I know you have been using VimTeX on Windows (since you wrote nice blog posts about this); would you be able and willing to test the new ideas there? I.e., try to use the new configuration suggested in :help vimtex-synctex-inverse-search with Vim and/or neovim and SumatraPDF?

lervag · 2021-10-25T20:48:49Z

@user202729 You're on Arch Linux and Vim, and I postulate that the current version should work well for you. Please criticise me! :)

clason · 2021-10-25T20:51:39Z

I know you already tried a couple of times before, @clason, but now I'm quite sure that things should work in general. There might be a minor issue if you open Skim in a strange way, although I don't understand why. I would appreciate feedback here.

I wish I had an answer. Getting the right environment for applications launched from the Finder is always a nightmare on macOS, and this seems to be the issue here. I'm a bit out of ideas (unless you can think of environment variables, especially $PATH, that VimTeX relies on here?)

In any case, this is not on you, so this shouldn't be a blocker.

lervag · 2021-10-25T20:59:54Z

I wish I had an answer. Getting the right environment for applications launched from the Finder is always a nightmare on macOS, and this seems to be the issue here. I'm a bit out of ideas (unless you can think of environment variables, especially $PATH, that VimTeX relies on here?)

It's strange - I can't really think of anything here. Perhaps it is more "severe"?

One thing you can do is to add things like call writefile(['lines'], '~/test.log', 'a') within the vimtex#view#inverse_search_comm(....) function definition to see if it is called. The same ideas can be used for debugging further. The debugging is clumsy, since we don't have an interactive vim session with captured output for doing the _comm call.

In any case, this is not on you, so this shouldn't be a blocker.

Ok, thanks; I agree, but I would like things to "just work"™ - if possible.

Btw: Does anyone have a better idea for the name of the function ...#inverse_search_comm? I don't quite like this name, but it is the best I've come up with so far...

lervag · 2021-10-25T21:00:27Z

@clason Did you test with both Vim and neovim - did things work with Skim as long as you started it from VimTeX (either directly or through latexmk)?

clason · 2021-10-25T21:05:24Z

I did not test with Vim, as I haven't used that in ages and don't have a compatible config anymore. Things work though as long as Skim is started from the shell (via VimTeX or open -a) rather than the Finder/Dashboard.

inverse_search_comm

inverse_search_remote?

user202729 · 2021-10-26T00:49:59Z

Seems to work well. Except

one should read |vimtex-synctex-inverse-search|. The interpolation variables for Zathura configuration are %{line} and %{file}, not %l and %f.

It's %{input} instead of %{file} in my version.

Btw: Does anyone have a better idea for the name of the function ...#inverse_search_comm? I don't quite like this name, but it is the best I've come up with so far...

send_inverse_search_commands?

Thanks, I understood that. My point was that I'm not really interested in supporting this part.

Okay, I don't really need it anyway.

lervag · 2021-11-04T22:00:12Z

Good idea. The alternative p.info['name'] in ('nvim', 'nvim.exe') looks better, though.

The disadvantage of this is that it does not include 'nvim-qt.exe', which we can add. I checked Neovide and it's process name is 'nvim.exe'. However, they are plenty more GUIs out there and we either have to check them and keep an up to date list or wait for someone to complain that it is not working with X GUI.

On the other hand 'nvim' in 'name' is too broad, however if we are going to check afterwards if it has $NVIM_LISTEN_ADDRESS or $NVIM_LISTEN_ADDRESS_VIMTEX or something else then I think we are good.

A compromise, then: let's do nvim in name when we also look for the env var, and then use the more explicit variant in the other case.

psutil is a standard lib, so I think in reality the dependency here is python3 and pynvim. And those seem quite common and I don't expect it to be considered a problem.

On my windows installation for both python 3.9 and 3.10, and fresh Conda installations 3.9 and 3.10 is not installed by default and must be installed.

Sorry, my mistake. You are right, psutil is not part of the standard libraries.

I do not consider them a problem either, however it should be mentioned on the README/doc for anyone interested. For example, I do not use any plugin with pynvim dependency so I would be wondering what is going on.

Agreed; however, I've been thinking about it, and I don't think we need pynvim here. We only need the python provider itself and psutil.

Yes, this reference seems to indicate that we can't properly resolve this right now. So, perhaps we could fall back to something inspired by @jdhao here. That is, instead of doing the python stuff, we can write the servernames to a file ...

That would remove the psutil and pynvim dependency which is nice but it would add reading and writing files which is I personally prefer even less.

I'm curious: why?

In any case, if you decide on this approach, neovim has stdpath('cache') (see :help stdpath) which can be used as a default.

Yes; I believe we can use the g:vimtex_cache_root location for this.

With this it works now.

The current version should include these patches and so I'll mark it as verified for neovim on Windows. Thanks!

Alternatively, we can check only for "NVIM_LISTEN_ADDRESS" and then compare the filename only as we already do. So strictly speaking we do not need "NVIM_LISTEN_ADDRESS_VIMTEX" in this scenario.

Btw, NVIM_LISTEN_ADDRESS has a note (Deprecated). Does this mean they will remove it completely in the future? Maybe using v:servername to assign NVIM_LISTEN_ADDRESS_VIMTEX is safer?

I think setting the environment variable is good. The deprecation notice at the very least indicates that we should not rely on NVIM_LISTEN_ADDRESS.

Notice, I tried to make the environment variable approach work on Linux, but here the p.environ() did not include these variables. It's strange, and I suspect it may be that the environment presented by p.environ() is not updated after the process itself has updated it.

Regarding the terminal popping up

cmd /c start /min "" nvim --headless -c "VimtexInverseSearch %l '%f'"

With this command the cmd still shows up but just for a moment before it hides again. It as very short flash.

From here: https://stackoverflow.com/a/67300159 Although the comment says "I've never seen a window flash when calling it via cmd /c start /min "". I am afraid it still does in my case. But it is the shortest one I managed to find and make it worrk.

Ok, I'll update the docs accordingly. I'm curious, did you try the start /b as previously suggested?

PanagiotisS · 2021-11-05T00:25:45Z

All look OK now for Neovim on Windows.

A compromise, then: let's do nvim in name when we also look for the env var, and then use the more explicit variant in the other case.

I am not sure what you mean for the other variant but the code look fine now!

Agreed; however, I've been thinking about it, and I don't think we need pynvim here. We only need the python provider itself and psutil.

I was thinking the same. And that is why I made a new conda environment to test it.
I tried again. Here is the terminal output

(base) PS C:\Users\ps> conda create --name test_pynvim
Collecting package metadata (current_repodata.json): done
Solving environment: done

## Package Plan ##

  environment location: C:\tools\miniconda3\envs\test_pynvim


Proceed ([y]/n)? y

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
#
# To activate this environment, use
#
#     $ conda activate test_pynvim
#
# To deactivate an active environment, use
#
#     $ conda deactivate

(base) PS C:\Users\ps> conda activate test_pynvim
(test_pynvim) PS C:\Users\ps> conda install python==3.9
Collecting package metadata (current_repodata.json): done
Solving environment: failed with initial frozen solve. Retrying with flexible solve.
Collecting package metadata (repodata.json): done
Solving environment: done



  added / updated specs:
    - python==3.9


The following NEW packages will be INSTALLED:

  ca-certificates    pkgs/main/win-64::ca-certificates-2021.10.26-haa95532_2
  certifi            pkgs/main/win-64::certifi-2021.10.8-py39haa95532_0
  openssl            pkgs/main/win-64::openssl-1.1.1l-h2bbff1b_0
  pip                pkgs/main/win-64::pip-21.2.4-py39haa95532_0
  python             pkgs/main/win-64::python-3.9.0-h6244533_2
  setuptools         pkgs/main/win-64::setuptools-58.0.4-py39haa95532_0
  sqlite             pkgs/main/win-64::sqlite-3.36.0-h2bbff1b_0
  tzdata             pkgs/main/noarch::tzdata-2021e-hda174b7_0
  vc                 pkgs/main/win-64::vc-14.2-h21ff451_1
  vs2015_runtime     pkgs/main/win-64::vs2015_runtime-14.27.29016-h5e58377_2
  wheel              pkgs/main/noarch::wheel-0.37.0-pyhd3eb1b0_1
  wincertstore       pkgs/main/win-64::wincertstore-0.2-py39haa95532_2


Proceed ([y]/n)? y

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
(test_pynvim) PS C:\Users\ps> conda install psutil
Collecting package metadata (current_repodata.json): done
Solving environment: done

## Package Plan ##

  environment location: C:\tools\miniconda3\envs\test_pynvim

  added / updated specs:
    - psutil


The following NEW packages will be INSTALLED:

  psutil             pkgs/main/win-64::psutil-5.8.0-py39h2bbff1b_1


Proceed ([y]/n)? y

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
(test_pynvim) PS C:\Users\ps> nvim --headless -c "VimtexInverseSearch 8 'C:\Users\ps\Documents\MyDocuments\test.tex'"
(test_pynvim) PS C:\Users\ps> conda install -c conda-forge pynvim
Solving environment: done

## Package Plan ##

  environment location: C:\tools\miniconda3\envs\test_pynvim

  added / updated specs:
    - pynvim


The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    certifi-2021.10.8          |   py39hcbf5309_1         145 KB  conda-forge
    greenlet-1.1.2             |   py39h415ef7b_1          92 KB  conda-forge
    msgpack-python-1.0.2       |   py39h2e07f2f_2          81 KB  conda-forge
    ------------------------------------------------------------
                                           Total:         319 KB

The following NEW packages will be INSTALLED:

  greenlet           conda-forge/win-64::greenlet-1.1.2-py39h415ef7b_1
  msgpack-python     conda-forge/win-64::msgpack-python-1.0.2-py39h2e07f2f_2
  pynvim             conda-forge/win-64::pynvim-0.4.3-py39hcbf5309_0
  python_abi         conda-forge/win-64::python_abi-3.9-2_cp39
  pyuv               conda-forge/win-64::pyuv-1.4.0-py39hb82d6ee_1003

The following packages will be UPDATED:

  certifi            pkgs/main::certifi-2021.10.8-py39haa9~ --> conda-forge::certifi-2021.10.8-py39hcbf5309_1

The following packages will be SUPERSEDED by a higher-priority channel:

  openssl              pkgs/main::openssl-1.1.1l-h2bbff1b_0 --> conda-forge::openssl-1.1.1l-h8ffe710_0


Proceed ([y]/n)? y


Downloading and Extracting Packages
certifi-2021.10.8    | 145 KB    | ##################################################################################################################################################################### | 100%
msgpack-python-1.0.2 | 81 KB     | ##################################################################################################################################################################### | 100%
greenlet-1.1.2       | 92 KB     | ##################################################################################################################################################################### | 100%
Preparing transaction: done
Verifying transaction: done
Executing transaction: done
(test_pynvim) PS C:\Users\ps> nvim --headless -c "VimtexInverseSearch 8 'C:\Users\ps\Documents\MyDocuments\cv\test.tex'"
['\\.\pipe\nvim-14000-0']
(test_pynvim) PS C:\Users\ps> conda remove pynvim
Collecting package metadata (repodata.json): done
Solving environment: done

## Package Plan ##

  environment location: C:\tools\miniconda3\envs\test_pynvim

  removed specs:
    - pynvim


The following packages will be REMOVED:

  greenlet-1.1.2-py39h415ef7b_1
  msgpack-python-1.0.2-py39h2e07f2f_2
  pynvim-0.4.3-py39hcbf5309_0
  python_abi-3.9-2_cp39
  pyuv-1.4.0-py39hb82d6ee_1003

The following packages will be UPDATED:

  ca-certificates    conda-forge::ca-certificates-2021.10.~ --> pkgs/main::ca-certificates-2021.10.26-haa95532_2

The following packages will be SUPERSEDED by a higher-priority channel:

  certifi            conda-forge::certifi-2021.10.8-py39hc~ --> pkgs/main::certifi-2021.10.8-py39haa95532_0
  openssl            conda-forge::openssl-1.1.1l-h8ffe710_0 --> pkgs/main::openssl-1.1.1l-h2bbff1b_0


Proceed ([y]/n)? y

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
(test_pynvim) PS C:\Users\ps> nvim --headless -c "VimtexInverseSearch 8 'C:\Users\ps\Documents\MyDocuments\test.tex'"
(test_pynvim) PS C:\Users\ps>

You can see the output of 165 unsilent echo l:socket_ids when is running ok (btw was it left intentionally there?)

That would remove the psutil and pynvim dependency which is nice but it would add reading and writing files which is I personally prefer even less.

I'm curious: why?

Not a strong preference honestly. Mostly comes from my frustration that ALL the applications dumping a file in my $HOME directory which then becomes a huge mess. There is a chaos there of files. (Which btw was a "huge" advantage of neovim as vim and the plugins I was using were also part of the problem). But jokes aside, if you can configure where the file will be then is fine.

Notice, I tried to make the environment variable approach work on Linux, but here the p.environ() did not include these variables. It's strange, and I suspect it may be that the environment presented by p.environ() is not updated after the process itself has updated it.

That is interesting! I suppose here is why https://unix.stackexchange.com/questions/29128/how-to-read-environment-variables-of-a-process

Ok, I'll update the docs accordingly. I'm curious, did you try the start /b as previously suggested?

I did try a lot of things and options for both cmd and nvim. I did not remember which exactly now.
Testing again the following:

cmd /c start /min "" nvim --headless -c "VimtexInverseSearch %l '%f'" : Instant flash
nvim --headless -c "VimtexInverseSearch %l '%f'": "long" popup cmd window
start /b "" nvim --headless -c "VimtexInverseSearch %l '%f'": does not work. When running from powershell terminal it is asking for "Windows PowerShell credential request" and is asking for username and password. When running from cmd terminal it works without popup (as expected).
cmd /c start /b nvim --headless -c "VimtexInverseSearch %l '%f'': Similar to 2. when run from SumatraPDF, however when run from the terminal (windows terminal with powershell) is instantaneous. Weird difference.
PowerShell.exe -windowstyle hidden {nvim --headless -c "VimtexInverseSearch %l '%f''"}: Instant flash. Similar speed to 1. when run from SumatraPDF. No popup when run from the powershell terminal. However, as the default size of powershell is bigger the flashing window is bigger. And blue instead of black. (Not that it matters much).

Adding cmd /q /q makes no difference.

So what I think happens in 1. is that we need to open cmd, in order to run start, which runs nvim. So the short flash is from cmd.

https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/start
https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/cmd

EDIT: Also, this command assumes nvim is in PATH obviously. Otherwise, the full path must be used. For example
cmd /c start /min "" "C:\tools\neovim\Neovim\bin\nvim.exe" --headless -c "VimtexInverseSearch %l '%f'"

BertrandSim · 2021-11-05T07:21:49Z

My previous comment about 'shellslash' was in relation to the use of globpath(). However, since globpath() is no longer used here, I think one can drop the 'shellslash' consideration altogether

I'm sorry, I'm a little bit confused.

So sorry for the confusion. I think part of the confusion stems from which "previous comment" I was referring to. (I have now linked to the correct "comment" in the quote above)

Summarizing that "previous comment" (and rephrasing),

with globpath() and 'shellslash', does not work
with globpath() and 'noshellslash', works
with \ and 'shellslash', works
with \ and 'noshellslash', works

Do I understand correctly that things work without doing the change you suggested, so that I can disregard it?

Not quite, without my suggested change, the inverse search only works when 'shellslash' is not set; it should also work when 'shellslash' is set.

I think one can drop the 'shellslash' consideration altogether

By this, I mean that one should always replace / with \ whether 'shellslash' is set or not (ie, always use \ for windows paths; there is no need to consider the 'shellslash' option), hence my suggestion here

I hope that clarifies, please write back to me otherwise :)

PanagiotisS · 2021-11-05T11:14:44Z

@BertrandSim

I was actually curious so I tried it vim on Windows.
It works with both set shellslash and set noshellslash as it is expected as this exists in the code

https://github.com/lervag/vimtex/pull/2219/files#diff-75ba36348a62468eaed3de511d31c8407867b60597d17d2de485c8de587ed10dR34-R36

so shellslash should be irrelevant. Have you tried pulling the latest changes?
The code does not use globpath.

Here is a minimal vimrc

if &compatible
  set nocompatible
endif

set runtimepath+=C:\Users\ps\.cache\dein\repos\github.com\Shougo\dein.vim

call dein#begin('C:\Users\ps\.cache\dein')
call dein#add('C:\Users\ps\.cache\dein\repos\github.com\Shougo\dein.vim')
call dein#add('lervag/vimtex', {'rev': 'feat/better-inverse-search'})
call dein#end()

filetype plugin indent on
syntax enable

set number
set shellslash

@lervag
Regarding cmd window with vim it actually has different behaviour.
Any combination of cmd and start (f.e. cmd /q /c start /b) leaves the cmd window open with the following message

♀
C:\Users\ps\AppData\Local\SumatraPDF>

No clue where this symbol comes from.

Using cmd /c vim -T dumb --not-a-term -n -c "VimtexInverseSearch %l '%f'" works fine although the cmd window stays open for <1sec. (longer than cmd /c start /min in neovim case which is almost instant).

Finally, now it works for me with both vim and neovim on Windows.

EDIT: P.S. Also from neobundle readme

Note: Active developement on NeoBundle has stopped. The only future changes will be bug fixes.

Please see Dein.vim -- A faster, well-tested plugin manager for Vim and Neovim. It can do everything NeoBundle does, including asynchronous installs.

BertrandSim · 2021-11-05T13:46:20Z

@PanagiotisS

so shellslash should be irrelevant.

Agreed, That was exactly my point. I guess I had not been clear enough, but that was what I was trying to say :S

The code does not use globpath

Again, that was exactly what I was getting at. The code does not use globpath, so there is no need to use shellslash in the code.

It works with both set shellslash and set noshellslash as it is expected as this exists in the code

No, I am afraid it does not work with shellslash set.
In the code

function! vimtex#paths#s(path) abort " {{{1
  " Handle shellescape issues and simplify path
  let l:path = exists('+shellslash') && !&shellslash
        \ ? tr(a:path, '/', '\')
        \ : a:path

  return simplify(l:path)
endfunction

It only makes the correction (tr(...)) if !&shellslash, ie. when shellslash is not set. tr(...) is not applied if &shellslash.

user202729 · 2021-11-05T14:46:05Z

Regarding start: it's a Windows cmd's built-in, that's the likely reason why it does not work in Sumatra PDF spawn.

As I mentioned above in the general case VBScript may be needed.

But for gvim in particular, it may be possible without VBScript:

I tried running some commands. The "nothing pops up" cases are highlighted.

gvim --remote a

(assuming there's a GVIM remote server) A terminal window pop up very quickly then exits

"C:\Program Files (x86)\Vim\vim82\gvim" --remote a

Nothing pops up

"C:\Program Files (x86)\Vim\vim82\vim" --remote a

(assuming there's a VIM remote server) A terminal pops up quickly then exits

"C:\Program Files (x86)\Vim\vim82\gvim" -c ":call mkdir('D:\a\' .. rand()) | quit"

Gvim window pop up quickly then exits

gvim --cmd ":call mkdir('D:\a\' .. rand()) | quit"

A terminal window pop up very quickly then exits

"C:\Program Files (x86)\Vim\vim82\gvim" --cmd ":call mkdir('D:\a\' .. rand()) | quit"

Nothing pops up

"C:\Program Files (x86)\Vim\vim82\vim" --cmd ":call mkdir('D:\a\' .. rand()) | quit"

A terminal window pop up (longer than gvim's terminal window) then exits

The cause of some of the popups is that gvim is shorthand for gvim.bat file the installer created to be able to use it from command-line.

So in conclusion

"C:\Program Files (x86)\Vim\vim82\gvim" --cmd "VimtexInverseSearch %l '%f'

is likely to work. (drop x86 on 32-bit Windows)

lervag · 2021-11-05T22:29:36Z

@PanagiootisS

All look OK now for Neovim on Windows.

Glad to hear it!

Agreed; however, I've been thinking about it, and I don't think we need pynvim here. We only need the python provider itself and psutil.

I was thinking the same. And that is why I made a new conda environment to test it. I tried again. Here is the terminal output

Thanks; I understand this as evidence that pynvim is required.

You can see the output of 165 unsilent echo l:socket_ids when is running ok (btw was it left intentionally there?)

No, sorry - I'll remove. Thanks for noticing!

That would remove the psutil and pynvim dependency which is nice but it would add reading and writing files which is I personally prefer even less.

I'm curious: why?

Not a strong preference honestly. Mostly comes from my frustration that ALL the applications dumping a file in my $HOME directory which then becomes a huge mess.

Well, I strongly relate to this frustration and I'll of course avoid messing with your home.

I plan to look into this idea in a follow-up PR after I merge this one.

That is interesting! I suppose here is why https://unix.stackexchange.com/questions/29128/how-to-read-environment-variables-of-a-process

Thanks, this seems to confirm my suspicion.

I did try a lot of things and options for both cmd and nvim. I did not remember which exactly now. Testing again the following: ...

I'm left with the impression that the current version of the docs are satisfactory, then. Is that right?

EDIT: Also, this command assumes nvim is in PATH obviously. Otherwise, the full path must be used. For example cmd /c start /min "" "C:\tools\neovim\Neovim\bin\nvim.exe" --headless -c "VimtexInverseSearch %l '%f'"

Yes, good point. But I would think it is not necessary to document this, it seems an edge case?

@PanagiootisS and @BertrandSim

so shellslash should be irrelevant. Have you tried pulling the latest changes?

No, I am afraid it does not work with shellslash set.

Again I'm slightly confused, but my understanding is that I should apply the patch suggested by @BertrandSim here. Do you both agree?

Regarding cmd window with vim it actually has different behaviour. ...

Using cmd /c vim -T dumb --not-a-term -n -c "VimtexInverseSearch %l '%f'" works fine although the cmd window stays open for <1sec.

@BertrandSim Is this the same on your end?

Finally, now it works for me with both vim and neovim on Windows.

Great, glad to hear it! This is quite close to ready, then!

@user202729

So in conclusion
"C:\Program Files (x86)\Vim\vim82\gvim" --cmd "VimtexInverseSearch %l '%f'
is likely to work. (drop x86 on 32-bit Windows)

Thanks; it seems the above cmd /c versions should also work well - would you still recommend to update/change/append something about this to the docs?

user202729 · 2021-11-06T01:57:43Z

Up to you. The disadvantage is that you need to know the "real" path to gvim to do it.

Alternatively add it (only) if many people ask for it.

PanagiotisS · 2021-11-06T10:55:32Z

I'm left with the impression that the current version of the docs are satisfactory, then. Is that right?

Github had the doc file folded so I missed. I though you were planning this last thing or something.

5257 cmd /c start /min "" nvim --headless -c "VimtexInverseSearch %l '%f'"

Based on my experience a vim example is needed. I suppose you are waiting for more feedback before updating this, but just in case I wanted to point it.

297 protocol in the same fashion as the clientserver. Both Vim and neovim has
298 the |v:servername| variable that contains the name/location of the server
299 with which we need to communicate.

Shouldn't this be "Both Vim and Neovim have" ?

*vimtex-faq-neovim*

We probably still need this for the "pynvim" part. And probably a reference from the *vimtex-requirements* session in a similar manner to windows reference would be really nice "and Neovim users should also read |vimtex-faq-neovim|"

EDIT: Also, this command assumes nvim is in PATH obviously. Otherwise, the full path must be used. For example cmd /c start /min "" "C:\tools\neovim\Neovim\bin\nvim.exe" --headless -c "VimtexInverseSearch %l '%f'"

Yes, good point. But I would think it is not necessary to document this, it seems an edge case?

Nowdays I am not sure as I do use mostly Linux. It really depends on how one installs (n)vim. In the past, I certainly had to edit PATH a lot of times and I still do every now and then. For example, I had to manually add mingw bin to the path a couple of days ago. And lets not start with Python and Windows and the huge mess that existed already, which Windows 10 made worse with their intrusive tactics. And (I am sorry for giving once again a personal example) considering that most people I know that they use latex (engineering, in academia) probably do not know what exactly PATH is or how it works (nor they care). So, having an explicit one-line example (or try"this" if it doesn't work) would be very helpful for them, if they ever came to use this plugin (some of them are using vim but without plugins). But then again I do not know. You can maybe leave it off and if someone asks you can add it later.

Regarding path and shellslash

@BertrandSim I am sorry. You are right. With shellslash it does not work. I messed it up apparently. (I did not take into account that half of the code is executed in a different (n)vim instance than the one we start with.)

@lervag

Again I'm slightly confused, but my understanding is that I should apply the patch suggested by @BertrandSim here. Do you both agree?

Yes. So it seems that regardless of shellslash setting if we do 'tr(a:path, '/', '')` we are good.

To see what is going on I will put a couple of echoms in the code and posting them here so you can see what's going on,

in autoload/vimtex/view.vim

function! vimtex#view#inverse_search(line, filename) abort " {{{1
  " Only activate in VimTeX buffers
  if !exists('b:vimtex') | return -1 | endif

  echom '-----------'
  echom 'Shellslash = ' . &shellslash

  " Only activate in relevant VimTeX projects
  let l:file = resolve(a:filename)
  let l:sources = copy(b:vimtex.sources)
  echom '1: l:file = ' . l:file
  if vimtex#paths#is_abs(l:file)
    call map(l:sources, {_, x -> vimtex#paths#join(b:vimtex.root, x)})
  endif
  echom '4: l:sources = ' . string(l:sources)
  if index(l:sources, l:file) < 0 | return -2 | endif

in autoload/vimtex/paths.vim

function! vimtex#paths#s(path) abort " {{{1
  " Handle shellescape issues and simplify path
  echom '2: a:path = ' . a:path
  " let l:path = exists('+shellslash') && &shellslash
        " \ ? tr(a:path, '/', '\')
        " \ : a:path
  let l:path = tr(a:path, '/', '\')
  echom '3: l:path = ' . l:path

  return simplify(l:path)
endfunction

And the result is (for both vim and neovim),

-----------
Shellslash = 1
1: l:file = C:\Users\ps\Documents\MyDocuments\test.tex
2: a:path = C:/Users/ps/Documents/MyDocuments/test.tex
3: l:path = C:\Users\ps\Documents\MyDocuments\test.tex
4: l:sources = ['C:\Users\ps\Documents\MyDocuments\test.tex']

-----------
Shellslash = 0
1: l:file = C:\Users\ps\Documents\MyDocuments\test.tex
2: a:path = C:\Users\ps\Documents\MyDocuments/test.tex
3: l:path = C:\Users\ps\Documents\MyDocuments\test.tex
4: l:sources = ['C:\Users\ps\Documents\MyDocuments\test.tex']

lervag · 2021-11-06T12:07:14Z

I'm left with the impression that the current version of the docs are satisfactory, then. Is that right?

Github had the doc file folded so I missed. ...

Thanks for the comments; I've updated the docs now.

vimtex-faq-neovim

We probably still need this for the "pynvim" part. And probably a reference from the *vimtex-requirements* session in a similar manner to windows reference would be really nice "and Neovim users should also read |vimtex-faq-neovim|"

Ok, thanks; I'll add/update that.

Nowdays I am not sure as I do use mostly Linux. It really depends on how one installs (n)vim. In the past, I certainly had to edit PATH a lot of times and I still do every now and then. For example, I had to manually add mingw bin to the path a couple of days ago. And lets not start with Python and Windows and the huge mess that existed already, which Windows 10 made worse with their intrusive tactics. And (I am sorry for giving once again a personal example) considering that most people I know that they use latex (engineering, in academia) probably do not know what exactly PATH is or how it works (nor they care). So, having an explicit one-line example (or try"this" if it doesn't work) would be very helpful for them, if they ever came to use this plugin (some of them are using vim but without plugins). But then again I do not know. You can maybe leave it off and if someone asks you can add it later.

I'll consider to add something here.

Regarding path and shellslash

@BertrandSim I am sorry. You are right. With shellslash it does not work. I messed it up apparently. (I did not take into account that half of the code is executed in a different (n)vim instance than the one we start with.)

Great, I'll fix and update. Thanks for looking into it so thoroughly!

I'll now take the liberty of merging this PR. Please don't hesitate to open issues with ideas for improving the docs or this new feature.

And again, thanks to everyone who took their time to pitch in! I very much appreciate it!

refer: #2219

lervag · 2021-11-06T12:37:52Z

Again, thank you all. This was more work than anticipated, but I think and hope it will make the experience better for inverse search with VimTeX.

lervag force-pushed the feat/better-inverse-search branch from b307ce5 to 90931d6 Compare October 25, 2021 13:31

user202729 reviewed Oct 25, 2021

View reviewed changes

autoload/vimtex/view/zathura.vim Outdated Show resolved Hide resolved

lervag force-pushed the feat/better-inverse-search branch from cee9005 to a27b31e Compare October 25, 2021 18:52

lervag added 10 commits November 4, 2021 21:57

doc: simplified and updated inverse search docs

782a63a

test: add some inverse_search tests

b3e73bf

wip: fix error in latexmk+zathura

0210be5

doc: fix typo

5ca74b6

wip: reorder functions

817615f

wip: rename function

518a574

wip: add VimtexInverseSearch command

885830f

fix: properly handle paths on windows

89c4dda

fix: -v argument necessary on MacVim

5ec02ef

fix: use different python magic on windows

1041219

lervag force-pushed the feat/better-inverse-search branch from f72393c to 1041219 Compare November 4, 2021 20:58

fix: improve nvim server lookup

d66dbc5

lervag added 2 commits November 4, 2021 23:00

doc: comment for Windows users

2535d24

chore: minor simplification

799f375

lervag added a commit that referenced this pull request Nov 6, 2021

merge: implement better inverse search

f80d359

refer: #2219

lervag closed this Nov 6, 2021

lervag deleted the feat/better-inverse-search branch November 6, 2021 12:36

lervag mentioned this pull request Nov 6, 2021

Simplify neovim servernames for inverse search #2233

Merged

ez4lionky mentioned this pull request Feb 12, 2023

Let g:vimtex_view_method = 'zathura' , but cannot open zathura after compilation. #2630

Closed

Implement better inverse search #2219

Implement better inverse search #2219

Conversation

lervag commented Oct 25, 2021 • edited Loading

lervag commented Oct 25, 2021

clason commented Oct 25, 2021 • edited Loading

clason commented Oct 25, 2021

lervag commented Oct 25, 2021

clason commented Oct 25, 2021 • edited Loading

user202729 commented Oct 25, 2021 • edited Loading

clason commented Oct 25, 2021

user202729 commented Oct 25, 2021

lervag commented Oct 25, 2021

lervag commented Oct 25, 2021

clason commented Oct 25, 2021

lervag commented Oct 25, 2021

user202729 commented Oct 25, 2021

lervag commented Oct 25, 2021

clason commented Oct 25, 2021

lervag commented Oct 25, 2021

clason commented Oct 25, 2021 • edited Loading

lervag commented Oct 25, 2021

clason commented Oct 25, 2021 • edited Loading

lervag commented Oct 25, 2021

lervag commented Oct 25, 2021

lervag commented Oct 25, 2021

clason commented Oct 25, 2021 • edited Loading

lervag commented Oct 25, 2021

lervag commented Oct 25, 2021

clason commented Oct 25, 2021

user202729 commented Oct 26, 2021 • edited Loading

lervag commented Nov 4, 2021

Regarding the terminal popping up

PanagiotisS commented Nov 5, 2021 • edited Loading

BertrandSim commented Nov 5, 2021 • edited Loading

PanagiotisS commented Nov 5, 2021 • edited Loading

BertrandSim commented Nov 5, 2021 • edited Loading

user202729 commented Nov 5, 2021 • edited by lervag Loading

lervag commented Nov 5, 2021

user202729 commented Nov 6, 2021

PanagiotisS commented Nov 6, 2021

Regarding path and shellslash

lervag commented Nov 6, 2021

Regarding path and shellslash

lervag commented Nov 6, 2021

lervag commented Oct 25, 2021 •

edited

Loading

clason commented Oct 25, 2021 •

edited

Loading

clason commented Oct 25, 2021 •

edited

Loading

user202729 commented Oct 25, 2021 •

edited

Loading

clason commented Oct 25, 2021 •

edited

Loading

clason commented Oct 25, 2021 •

edited

Loading

clason commented Oct 25, 2021 •

edited

Loading

user202729 commented Oct 26, 2021 •

edited

Loading

PanagiotisS commented Nov 5, 2021 •

edited

Loading

BertrandSim commented Nov 5, 2021 •

edited

Loading

PanagiotisS commented Nov 5, 2021 •

edited

Loading

BertrandSim commented Nov 5, 2021 •

edited

Loading

user202729 commented Nov 5, 2021 •

edited by lervag

Loading