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

Change window focus behaviour on \ll: Focus the newly created viewer window, not vim #180

Closed
nasenatmer opened this issue Jun 7, 2015 · 30 comments

Comments

@nasenatmer
Copy link
Contributor

First, thanks for your great work on this project again :)

Although we talked about it recently and said it would be more intuitive if vimtex returns window focus to vim after the viewer has been started due to successful compilation by \ll, I think I've changed my view here.

Whenever I have a new window in front of any other window, I assume that this is the one in focus and start using key strokes (eg with zathura in order to set width, scroll down or something like that). However, these keys are sent to vim as the focus is returned to it when the viewer is opened for the firmt time, which often irritates me.

Although I'd appreciate if the default behaviour would change, I'd equally be happy to know which part of vimtex#util#execute needs changed in my personal fork so I can change this behaviour locally.

@lahwaacz
Copy link
Contributor

lahwaacz commented Jun 7, 2015

Wait, the vim window is focused but it still remains in "background"? This sounds more like a problem with the window manager...

@nasenatmer
Copy link
Contributor Author

Indeed. Workflow is as follows:

  1. Open vim to edit tex file
  2. press \ll, compilation starts
  3. when compilation has ended successfully, zathura opens
  4. openbox returns focus to vim, while zathura is still in the foreground

Did I mis-configure something with openbox? Here is my rc.xml, the only thing I remember to have configured that is related to this, is that I want mouse cursor to focus a window and to place new windows under mouse cursor.

@lahwaacz
Copy link
Contributor

lahwaacz commented Jun 7, 2015

I'm not sure, I haven't used Openbox for quite some time now... Perhaps it's disabled by the raiseOnFocus value? But allegedly the windows would be raised even as you move the mouse, which is probably not what you want.

Anyway I think that the focus should be changed only as new windows open, so ideally vimtex itself would never set focus. Unfortunately starting Zathura again to sync when the compilation finishes means that it will get focus, which is then set back to vim by the callback in vimtex. This also means that focus on a third window will not be preserved, so isn't there a way to sync the PDF viewer without starting the viewer again, which would also let the window manager manage the focus?

lervag added a commit that referenced this issue Jun 7, 2015
@lervag
Copy link
Owner

lervag commented Jun 7, 2015

I've not directly addressed this issue yet, but I've pushed a patch that should fix some minor itches. Could you please test the latest version and see if it helps somewhat?

@nasenatmer
Copy link
Contributor Author

Wow, so much replies here, but I first have to apologise, it seems I didn't make my problem clear enough.

The whole scenario I was talking about was only about the first time a viewer is opened by vimtex' \ll command.

I was not concerned with the behaviour of \lv.

Thanks for the update, @lervag, in the current version, \ll opens the viewer after successful compilation and then refocuses and raises the vim window. I clearly prefer that in comparison to the earlier behaviour and I think it is a great compromise!

However, as of now it seems that \lv does not get whether a viewer is open already and opens a second (or third or fourth) instance of it.

after \ll, \li output is as follows

b:vimtex : mwe
  root : '/home/jakob/tmp/latex/mwe'
  aux : '/home/jakob/tmp/latex/mwe/mwe.aux'
  log : '/home/jakob/tmp/latex/mwe/mwe.log'
  out : '/home/jakob/tmp/latex/mwe/mwe.pdf'
  tex : '/home/jakob/tmp/latex/mwe/mwe.tex'
  tmp : '/tmp/jakob/vG4ctdf/1'
  pid : '10107'
  cmd_latexmk_compile : 'cd ''/home/jakob/tmp/latex/mwe'' && max_print_line=2000 latexmk -pdf -e ''$pdflatex =~ s/ / -file-line-error /'' -pvc -
e ''$success_cmd = "vim --servername  --remote-expr \"vimtex\#latexmk\#callback(1)\""'' -e ''$failure_cmd = "vim --servername  --remote-expr \"v
imtex\#latexmk\#callback(0)\""'' -e ''$new_viewer_always = "0"'' -e ''$pdf_update_method = "0"'' -e ''$pdf_previewer = "start xdg-open "'' ''mwe
.tex'' >/tmp/jakob/vG4ctdf/1 2>&1'
  base : 'mwe.tex'
  viewer
    init : function('6')
    view : function('7')
    latexmk_append_argument : function('8')

and xdotool search -class Zathura returns:

48234498
48234500

After I do \lv, see a second zathura open, \li returns:

b:vimtex : mwe
  root : '/home/jakob/tmp/latex/mwe'
  aux : '/home/jakob/tmp/latex/mwe/mwe.aux'
  log : '/home/jakob/tmp/latex/mwe/mwe.log'
  out : '/home/jakob/tmp/latex/mwe/mwe.pdf'
  tex : '/home/jakob/tmp/latex/mwe/mwe.tex'
  tmp : '/tmp/jakob/vG4ctdf/1'
  pid : '10107'
  cmd_latexmk_compile : 'cd ''/home/jakob/tmp/latex/mwe'' && max_print_line=2000 latexmk -pdf -e ''$pdflatex =~ s/ / -file-line-error /'' -pvc -
e ''$success_cmd = "vim --servername  --remote-expr \"vimtex\#latexmk\#callback(1)\""'' -e ''$failure_cmd = "vim --servername  --remote-expr \"v
imtex\#latexmk\#callback(0)\""'' -e ''$new_viewer_always = "0"'' -e ''$pdf_update_method = "0"'' -e ''$pdf_previewer = "start xdg-open "'' ''mwe
.tex'' >/tmp/jakob/vG4ctdf/1 2>&1'
  base : 'mwe.tex'
  viewer
    cmd_view : 'xdg-open  ''/home/jakob/tmp/latex/mwe/mwe.pdf'''
    init : function('6')
    view : function('7')
    latexmk_append_argument : function('8')

xdotool search -class Zathurayields:

48234498
54525954
48234500
54525956

Something in the callback seems to not get the pid of the viewer window correctly?

lervag added a commit that referenced this issue Jun 8, 2015
@lervag
Copy link
Owner

lervag commented Jun 8, 2015

It seems my work yesterday was not tested properly. I don't really know/understand exactly why it failed, but I found a simple fix that seems to work well.

So, right now it seems things work mostly as expected. The question is then: Is the expected behaviour now good, or should it change. Perhaps I should add an option?

@nasenatmer
Copy link
Contributor Author

Great, \li prints the correct pid of zathura and \lv doesn't open additional windows now. The latest fix seems to have solved this issue now!

I'm quite happy with window behaviour after pressing \ll in the current state: \ll opens the viewer, but returns focus to vim and also raises vim so it is before the viewer window.

Personally, I'm not yet happy with \lv: \lv focuses the viewer window, but without raising it. This means that at least in my setup (irrespective of whether I set the mouse to be able to focus windows or not), focus is set on a window that may not be visible (because vim is in the foreground) since I always have a maximised terminal window on each desktop.

In my view, there are two possibilities:

  1. \lv just does forward search without focusing the viewer. In this case, no window events whatsoever happen.
  2. \lv does forward search and focuses the viewer window. In this case, the viewer window should also be raised so it becomes visible if it was behind any other window.

But this is just how I see it, I'd be more than happy if other users would contribute as to how they feel windows should behave…

lervag added a commit that referenced this issue Jun 8, 2015
@lervag
Copy link
Owner

lervag commented Jun 8, 2015

I guess the raise behaviour is different between different window managers. For consistency, I've added a raise for the viewer focus function, which in practice implements your second possibility.

Now it remains to decide what is the best default behaviour, and if it should be optional. Personally I find the current behaviour satisfying, and as such I propose to leave this issue as solved.

@Konfekt
Copy link
Contributor

Konfekt commented Jun 9, 2015

Leaving focus in Vim is fine, but the viewer window should become visible, that's been an itch for me. So how about

\lv does forward search and focuses the viewer window so that it becomes visible. Then returns the focus to Vim.

@nasenatmer
Copy link
Contributor Author

@Konfekt: I'm not sure whether I get you correctly: By „becoming visible“ you mean to raise the viewer to the front? And then want to return focus to vim?

If I am ricght, this means that if you have a small screen where you can only have one window in the front at a time, you have the focus on vim while the viewer window is in front and the only one visible? To me, this doesn't sound sensible.

Or do you mean "viewer visible" as in "not minimised, but still behind vim"?

As of now, I'm fully happy with window behaviour and think this would be a sane default. But this is of course only my stance.

@Konfekt
Copy link
Contributor

Konfekt commented Jun 9, 2015

Yeah, I meant that the focus rests in Vim but the viewer is made visible if it is not partially or fully covered by the current Vi instance but by any other window.

@lahwaacz
Copy link
Contributor

lahwaacz commented Jun 9, 2015

I don't think you can get a sensible default behaviour from vimtex without making assumptions on the window manager behaviour. For example in Openbox "focus a window" does not imply "raise a window", which I thought was true for most of the floating window managers. Then there are tiling window managers, which will most likely twist many of the assumptions. For exmaple in i3, opening a window which is configured to open on specified workspace does not move focus to the window, unless the target workspace is "visible", i.e. the "currently active" workspace or a workspace assigned to an external monitor.

I think there are only two sensible alternatives for vimtex:

  1. don't change focus at all and let the window manager do its job,
  2. or make it configurable.

As for no. 2, perhaps it's time to finally unify the viewer interface and expose the commands itself for configuration? I think they could be format strings where vimtex would replace the relevant information (path to file, synctex info etc).

@lervag
Copy link
Owner

lervag commented Jun 9, 2015

I am very happy for all the activity and interest here!

So, first: I agree with @nasenatmer that the current state is a sane default.

Based on the other opinions, I see that options might be a good idea. I have an idea that might just work: What if I add function hooks after the view and the callback functions. I would then keep the same default behaviour using the current focus settings, but these would be contained in default hooks. To customize the behaviour, one would then attach ones own hook functions. This will also provide a high level of flexibility (although I don't see how this can be used just now).

An example custom configuration (I've already committed a prototype implementation of this that works):

let g:vimtex_view_zathura_hook_view = 'MyHook'

function! MyHook() dict
  if self.xwin_id > 0
    silent call system('xdotool windowraise ' . self.xwin_id)
  endif
endfunction

For some window managers, this would ensure that the viewer window is raised (but not focused) after doing a forward search. Note that in fluxbox (which is currently my WM), this does not seem to do anything. That might also be due to personal settings on focus behaviour. I don't know.

Note that the current implementation allows such hooks for any viewer. The callback functionality is only available for MuPDF and Zathura, but the view hook may be added for any viewer.

@lervag
Copy link
Owner

lervag commented Jun 9, 2015

Btw, @lahwaacz, I would be very happy to implement a more unified system. I don't really see exactly how to do this, though. The current system is IMHO pretty OK, in particular because it sets sane defaults for an increasing number of viewers. It also implements some special code for selected viewers, such as MuPDF (hack for forward and backward search).

@lahwaacz
Copy link
Contributor

I don't exactly understand how the forward-search for MuPDF or Zathura work as I don't use any of those, but in principle it should be possible to write a wrapping shell script with interface similar to how qpdfview or Okular work (note especially the --unique flag). There is already some grep/sed/xdotool black magic happening, so I think moving the logic to a shell script would only make things more transparent. Vimtex could provide a sample script in its directory, which users could copy into their ~/bin/ and configure the path in vimtex. There could be either multiple smaller scripts per-viewer or a single (I'd say bloated) script with a switch selecting the viewer to use.

To preserve the current behaviour for MuPDF, only the following information are necessary to be passed to the script:

  • path to .pdf file
  • path to .tex file
  • current line number
  • current column number
  • Vim's window ID

Zathura also utilizes v:progname and v:servername. Anything else could be done in the script I think.

@lervag
Copy link
Owner

lervag commented Jun 10, 2015

Nah, I don't think shell scripts is the way to go. The actual shell commands needed for the viewer are few and simple. The problem with supporting many viewers is that the commands differ. There are some more complexities, but these are not more easily solved with a shell script IMHO. Some of the complexities for a given viewer are:

  • The chosen viewer should be passed to latexmk, preferably with embedded backward search setting.
  • If the chosen viewer handles forward search, this must be handled.
  • MuPDF does not support forward or backward search, but there exists hacks that sort of work. This requires "black magic".
  • Zathura DOES support both forward and backward search, but requires black magic to prevent multiple instances (it lacks the --unique option).
  • Etc.

The current implementation is good in that it really is easy to add new viewers, as long as I know the proper commands for supporting forward and/or backward search.

Note here that evince is one of the few exceptions, since it only communicates via dbus for synctex. I have still not found a simple way to add forward and backward search for evince without using semi-advanced shell scripts or python code. That is why I have not implemented support for evince. I hope to add evince some time, but I need help to find a proper way to implement it. Else I think most viewers should be possible to support.

@lervag
Copy link
Owner

lervag commented Jun 10, 2015

Btw, since @nasenatmer is happy with the current default, and since I have implemented functionality to change the default behaviour, I decide to close this issue now. I don't mind to continue the discussion, though.

Please also feel free to open a new thread if you have a concrete idea of how to improve the view interface.

@lervag lervag closed this as completed Jun 10, 2015
@lahwaacz
Copy link
Contributor

I'm sorry, but running a 5-line shell script from vim with 5 parameters is IMHO simpler than assembling a command in vim, parsing its output and based on the result deciding which of the alternative commands shall be run next. Or perhaps we refer to different definition of simple? Next week I might try to prove with a concrete code that it is indeed simpler.

The script approach has also other advantages:

  • its path can be configured for latexmk's $pdf_viewer
  • makes the "black magic" more transparent and readable for other users
  • assuming that the path of the script is configurable, users would be able to use custom scripts with custom dependencies (and easily change the behaviour related to focusing of windows, for instance)
  • the script could be reused outside of vim/vimtex (I understand this is not the goal, but still: do one thing and do it well?)

So far I can see only a philosophical disadvantage of a vim plugin providing and relying on shell scripts...

@lervag
Copy link
Owner

lervag commented Jun 10, 2015

We would need to add multiple scripts, one for each viewer. Perhaps two for
viewers that work on both Linux and windows.

In any case, I am not convinced. But I will be happy to reconsider if you
can provide a good example.

ons. 10. jun. 2015, 16.00 skrev Jakub Klinkovský notifications@github.com:

I'm sorry, but running a 5-line shell script from vim with 5 parameters is
IMHO simpler than assembling a command in vim, parsing its output and based
on the result deciding which of the alternative commands shall be run next.
Or perhaps we refer to different definition of simple? Next week I
might try to prove with a concrete code that it is indeed simpler
https://wiki.archlinux.org/index.php/The_Arch_Way#Simplicity.

The script approach has also other advantages:

  • its path can be configured for latexmk's $pdf_viewer
  • makes the "black magic" more transparent and readable for other users
  • assuming that the path of the script is configurable, users would be
    able to use custom scripts with custom dependencies (and easily change the
    behaviour related to focusing of windows, for instance)
  • the script could be reused outside of vim/vimtex (I understand this
    is not the goal, but still: do one thing and do it well?)

So far I can see only a philosophical disadvantage of a vim plugin
providing and relying on shell scripts...


Reply to this email directly or view it on GitHub
#180 (comment).

@lahwaacz
Copy link
Contributor

Does vim support string formatting similar to these examples? Or is it possible to achieve this behaviour some other way? If there were a configuration variable such as

let g:vimtex_viewer_command = "viewer_wrapper --pdf %pdf% --tex %tex% --line %line% --column %column%"

where vimtex would replace %foo% with the actual value, it would be possible to configure most of the viewers directly, thus having wrapper scripts only for MuPDF and Zathura.

@lervag
Copy link
Owner

lervag commented Jun 10, 2015

Vim does not support string formatting like that, but it is "easy" to implement it with substitutes. The question here is which kinds of configuration variables one needs.

Some viewers require a seperate command for performing forward search. And some (MuPDF) require more or less complicated scripts.

But I do like the idea of defining a more general way to implement the viewer which requires less code. Then we could still provide complete templates for several viewers, but it would be much easier to customize.

I think a top-down approach might be interesting here: What possible settings could a user want to set? Obviously, most users should be satisfied with something like g:vimtex_view_method, which should support several viewers out of the box. But some users might want to support new viewers or do some other customization.

I'm out of time, but I think we might be on to something interesting here.

@lervag lervag reopened this Jun 18, 2015
@nasenatmer
Copy link
Contributor Author

Still unsatisfied with window behaviour?

@lervag
Copy link
Owner

lervag commented Jun 22, 2015

The issue is still open because I want to investigate whether I can create an improved implementation of the view interface. I want to try to use @lahwaacz's idea to use placeholders. I think this could reduce the implementation to a single set of view functions, and then instead use options (with a predefined set of options for the currently supported viewers) to implement the view commands and forward search and similar.

lervag added a commit that referenced this issue Jun 30, 2015
This update removes the viewers

  qpdfview
  sumatrapdf
  skim
  okular

from the possible g:vimtex_view_method values.  Instead, these viewers
may now be defined with the `general` view method using the options

  g:vimtex_view_general_viewer
  g:vimtex_view_general_options
  g:vimtex_view_general_options_latexmk

Resolves: #180
@lervag
Copy link
Owner

lervag commented Jun 30, 2015

I've finally come around to this one. I've pushed an update where most of the viewers are now defined through a generic interface. Please see the update documentation, :h vimtex_viewer_general.

Comments are appreciated. I think this update makes sense.

@lahwaacz

@lervag lervag closed this as completed Jun 30, 2015
@lahwaacz
Copy link
Contributor

lahwaacz commented Jul 3, 2015

Hi, the update is a great step, thanks! The configuration for qpdfview (and likely others) suggested in the docs works just fine, in a few days I might finally look at the MuPDF/Zathura wrappers as I mentioned above.

@lervag
Copy link
Owner

lervag commented Jul 4, 2015

Thanks. I would be happy to consider a different approach to Zathura/MuPDF, but I have to admit that I find the current implementation quite OK. I also consider it to have a couple of benefits:

  1. A lot of the functionality for Zathura and MuPDF is common, and thus share code.
  2. Since everything is done in vimL and as separate viewer methods, we have very good control and flexibility. That is, it is easy to debug and it is easy to add/change functionality/features.

An alternative approach should at least provide similar benefits, and in order for me to want to change stuff, it should bring either simplifications or other obvious improvements.

@lahwaacz
Copy link
Contributor

lahwaacz commented Jul 4, 2015

I've been trying MuPDF and Zathura today and poked around the black magic in vimtex ;) Is it fine to continue the discussion here or should I open a new issue? I don't have a pull request yet though.

As far as I can tell, the only thing preventing the configuration of Zathura via the new general approach is that its --synctex-forward expects an instance of Zathura to be already running. And of course some additional placeholders would be necessary in order to preserve the current behaviour. The docs say

Forward search requires `xdotool` to work properly, in order to
not open a new Zathura instance for invocation of |VimtexView|.

but it seems that xdotools is used only to check if Zathura is already running. Perhaps it has been updated lately?

@lahwaacz
Copy link
Contributor

lahwaacz commented Jul 4, 2015

I've submitted a feature request for zathura: https://bugs.pwmt.org/issue487

Regarding MuPDF, I've run against some very serious bugs in xdotool, it can't even parse the command line properly. If it worked as it's supposed to (and documented), most of the MuPDF helper functions in vimtex could be reduced to just a couple of commands.

@lervag
Copy link
Owner

lervag commented Jul 7, 2015

I think it is OK to continue the discussion here. No problem :)

I agree that the only thing that forces me to keep the "black magic" for Zathura is that --synctex-forward expects an instance of Zathura to be running. xdotools is used to detect if Zathura is running. This is needed, because we use a different command for starting Zathura than the one we use for forward search.

It would of course be very nice if Zathura is updated. Similarly it would be very nice if MuPDF worked as it's supposed to. Still, even if both of these are patched, the updates would only be valid for those of us who use very recent versions of software. I guess we could simplify vimtex and say that the full functionality will only be available if the user uses updated versions, but that is somewhat rude...

@lervag
Copy link
Owner

lervag commented Jul 7, 2015

I've changed my mind: It is much easier to keep track of active discussions if they are open issues. And since we're really off topic from the present issue, it would be more suitable to continue the discussion in a new issue. I hope that's fine by you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants