Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to SyncTeX backward search #2135

Closed
wants to merge 3 commits into from
Closed

Conversation

psvenk
Copy link
Contributor

@psvenk psvenk commented Aug 17, 2021

Fixes #2127. This pull requests addresses three closely related issues related to SyncTeX backward search in multi-file projects, the last of which is the one originally brought up in #2127:

  • The latexmk command line, invoked upon <LocalLeader>ll, opens Zathura with --remote for backward search, while running <LocalLeader>lv opens Zathura with --remote-expr for backward search. This can lead to inconsistent behavior; 2139057 fixes this by properly escaping all arguments in the latexmk command line in order to use --remote-expr.
  • Backward search does not properly take into account the possibility that a file is open in another buffer, but not in the current tab page. This was fixed in 9af6610.
  • The function vimtex#view#reverse_goto, which is called upon backward search, is hard-coded to use edit to open a new file, usurping the current window; b003fee makes this configurable and uses bufloaded instead of bufexists to avoid falling back to usurpation when a buffer exists but is not visible (e.g., if it were opened in another tab page and then closed, we want to reopen it in a tab page — if this is the user's preference — and not always try to usurp the current window to load that buffer).

In the latexmk command line, make sure that --remote-expr is used
instead of --remote for SyncTeX backwards search when opening Zathura.
This ensures consistency between starting compilation via
<LocalLeader>ll and opening the viewer separately via <LocalLeader>lv.

Refer lervag#2127 (comment)
When a file is open in a different tab page, make sure to switch to that
tab page when SyncTeX backward search requests that file. The previous
behavior was to use bufwinnr, which does not look at windows in other
tab pages and therefore acts as if the buffer is not open anywhere.

Refer https://stackoverflow.com/a/42628934
This option controls the command that is used to open new files from
SyncTeX backward search in multi-file projects.

Refer lervag#2127
@lervag
Copy link
Owner

lervag commented Aug 17, 2021

Fixes #2127. This pull requests addresses three closely related issues related to SyncTeX backward search in multi-file projects, the last of which is the one originally brought up in #2127:

Thanks!

The latexmk command line, invoked upon <LocalLeader>ll, opens Zathura with --remote for backward search, while running <LocalLeader>lv opens Zathura with --remote-expr for backward search. This can lead to inconsistent behavior; 2139057 fixes this by properly escaping all arguments in the latexmk command line in order to use --remote-expr.

Wow, impressive level of shell escape magic!

Backward search does not properly take into account the possibility that a file is open in another buffer, but not in the current tab page. This was fixed in 9af6610.

Great, thanks!

The function vimtex#view#reverse_goto, which is called upon backward search, is hard-coded to use edit to open a new file, usurping the current window; b003fee makes this configurable and uses bufloaded instead of bufexists to avoid falling back to usurpation when a buffer exists but is not visible (e.g., if it were opened in another tab page and then closed, we want to reopen it in a tab page — if this is the user's preference — and not always try to usurp the current window to load that buffer).

Sounds good!

execute l:tabnr . 'tabnext'
execute l:winnr . 'wincmd w'
catch
execute g:vimtex_view_reverse_search_edit_cmd l:file
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why we need the second execute ... (which is the same as line 78)? Is it because of the if !bufloaded?

Copy link
Contributor Author

@psvenk psvenk Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. I included this to replicate the behavior of the old line 94, which used buffer to load the buffer if it exists but is not loaded. With the change from bufexists to bufloaded, this would load the buffer if it is loaded but for whatever reason win_findbuf does not find it. From my limited knowledge, it seems that this should not happen, but I think I found a counterexample in my testing (though I don't remember what the situation was).

I changed buffer to the value of g:vimtex_view_reverse_search_edit_cmd to avoid usurping the current buffer in this case (if that is the user's preference). With default settings, edit is used instead of buffer and (as far as I know) the user experience is largely unchanged.

\ ? l:winnr . 'wincmd w'
\ : 'buffer ' . l:bufnr
" Get window and tab numbers
try
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would the following code fail? During tabnext? Or wincmd?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would actually fail in line 95; if the buffer does not exist, then win_findbuf returns an empty list and the unpacking fails.

@@ -80,7 +80,7 @@ function! s:zathura.latexmk_append_argument() dict abort " {{{1
if self.has_synctex
let zathura .= ' -x \"' . g:vimtex_compiler_progname
\ . ' --servername ' . v:servername
\ . ' --remote +\%{line} \%{input}\" \%S'
\ . ' --remote-expr \"\\\"\"vimtex#view#reverse_goto(\%{line}, ''"''"''\%{input}''"''"'')\"\\\"\"\" \%S'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hahahaha! This makes my day! :D

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the worst part of this is that it works! Great work!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I had to do quite a bit of testing with latexmk on the command line. For the record, this is what I used:

latexmk -pvc -new-viewer -pdf lectures.tex -e '$pdf_previewer = "zathura -x \"/usr/bin/vim.gtk3 --servername VIM --remote-expr \"\\\"\"vimtex#view#reverse_goto(\%{line}, '"'"'\%{input}'"'"')\"\\\"\"\" lectures.pdf"'

The output contained the command line used by latexmk to start Zathura, which I could cross-check against the use of --remote-expr already in zathura.vim:

Running 'start zathura -x "/usr/bin/vim.gtk3 --servername VIM --remote-expr "\""vimtex#view#reverse_goto(%{line}, '%{input}')"\""" lectures.pdf'

@lervag
Copy link
Owner

lervag commented Aug 17, 2021

I'll merge after feedback on my questions.

lervag added a commit that referenced this pull request Aug 19, 2021
@lervag
Copy link
Owner

lervag commented Aug 19, 2021

Thanks for the good work! I made some minor adjustments and merged this just now.

@lervag lervag closed this Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zathura backward search: bug with multi-file projects
2 participants