From 5671b1aed1628ea9ff47f966eb3b573b0705ca01 Mon Sep 17 00:00:00 2001
From: Elijan Mastnak <ejmastnak@gmail.com>
Date: Mon, 20 Jun 2022 11:57:03 +0200
Subject: [PATCH 1/4] feat: create atomic functions for X11 winID search

This commit adds three new functions:

- `xdo_find_win_id_by_class`,
- `xdo_find_win_id_by_name`, and
- `xdo_find_win_id_by_pid`,

which will later be called from `xdo_exists` and `xdo_get_id` for the
purposes of identifying a viewer's X11 window ID.
Motivation: similar winID-finding logic is used by both `xdo_exists` and
`xdo_get_id`, so it makes sense to outsource this to "atomic" functions
to avoid code repetition.

TODO: refactor `xdo_exists` and `xdo_get_id` to actually use the new
functions.
---
 autoload/vimtex/view/_template.vim | 55 ++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/autoload/vimtex/view/_template.vim b/autoload/vimtex/view/_template.vim
index f9762fb7f5..035e0e8c2e 100644
--- a/autoload/vimtex/view/_template.vim
+++ b/autoload/vimtex/view/_template.vim
@@ -188,6 +188,61 @@ function! s:viewer.xdo_exists() dict abort " {{{1
 endfunction
 
 " }}}1
+" Begin new code
+" --------------------------------------------- "
+" Attempts to find the viewer's X window ID using `xdotool` search by window
+" class. Returns the viewer's window ID if one is found, and `0` otherwise.
+function! s:viewer.xdo_find_win_id_by_class() dict abort " {{{1
+  let l:xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
+  if len(l:xwin_ids) == 0
+    " TODO: probably move logging to the higher-level functions `xdo_exists`
+    " and `xdo_get_id`
+    call vimtex#log#warning('Viewer cannot find ' . self.name . ' window ID!')
+    return 0
+  else
+    return l:xwin_ids[-1]
+  endif
+endfunction
+
+" }}}1
+" Attempts to find the viewer's X window ID using `xdotool` search by window
+" name (i.e. the string in the window titlebar). Returns the viewer's window
+" ID if one is found, and `0` otherwise.
+function! s:viewer.xdo_find_win_id_by_name() dict abort " {{{1
+  let l:xwin_ids = vimtex#jobs#capture(
+        \ 'xdotool search --name ' . fnamemodify(self.out(), ':t'))
+  let ids_already_used = filter(map(
+        \   deepcopy(vimtex#state#list_all()),
+        \   {_, x -> get(get(x, 'viewer', {}), 'xwin_id')}),
+        \ 'v:val > 0')
+  for id in l:xwin_ids
+    if index(ids_already_used, id) < 0
+      return id
+    endif
+  endfor
+  " If loop exits with no window ID found
+  return 0
+endfunction
+
+" }}}1
+" Attempts to find the viewer's X window ID using `xdotool` search by the
+" viewer's process ID. Returns the viewer's window ID if one is found, and `0`
+" otherwise.
+function! s:viewer.xdo_find_win_id_by_pid() dict abort " {{{1
+  let l:pid = has_key(self, 'get_pid') ? self.get_pid() : 0
+  if l:pid > 0
+    let xwin_ids = vimtex#jobs#capture(
+          \   'xdotool search --all --pid ' . l:pid
+          \ . ' --name ' . fnamemodify(self.out(), ':t'))
+    return get(xwin_ids, 0)
+  else
+    return 0
+  endif
+endfunction
+
+" }}}1
+" --------------------------------------------- "
+" End new code
 function! s:viewer.xdo_send_keys(keys) dict abort " {{{1
   if !self.xdo_check() || empty(a:keys) || self.xwin_id <= 0 | return | endif
 

From c5f5519dbae3ac01f2c067273ac0c281423f1b25 Mon Sep 17 00:00:00 2001
From: Elijan Mastnak <ejmastnak@gmail.com>
Date: Mon, 20 Jun 2022 11:59:05 +0200
Subject: [PATCH 2/4] refactor: make `xdo_exists` use dedicated winID-finding
 functions

This commit outsources the existing X11 window ID finding code in the
`xdo_exists` function to the dedicated functions
`xdo_find_win_id_by_pid` and `xdo_find_win_id_by_pid` introduced
previously in commit c036aa5.

There should be no externally visible change in VimTeX's behavior as a
result of this commit.
---
 autoload/vimtex/view/_template.vim | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/autoload/vimtex/view/_template.vim b/autoload/vimtex/view/_template.vim
index 035e0e8c2e..7f3d49f686 100644
--- a/autoload/vimtex/view/_template.vim
+++ b/autoload/vimtex/view/_template.vim
@@ -152,7 +152,7 @@ endfunction
 function! s:viewer.xdo_exists() dict abort " {{{1
   if !self.xdo_check() | return v:false | endif
 
-  " If xwin_id is already set, check if it still exists
+  " If xwin_id is already set, check if a matching viewer window still exists
   if self.xwin_id > 0
     let xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
     if index(xwin_ids, self.xwin_id) < 0
@@ -162,25 +162,13 @@ function! s:viewer.xdo_exists() dict abort " {{{1
 
   " If xwin_id is unset, check if matching viewer windows exist
   if self.xwin_id == 0
-    let l:pid = has_key(self, 'get_pid') ? self.get_pid() : 0
-    if l:pid > 0
-      let xwin_ids = vimtex#jobs#capture(
-            \   'xdotool search --all --pid ' . l:pid
-            \ . ' --name ' . fnamemodify(self.out(), ':t'))
-      let self.xwin_id = get(xwin_ids, 0)
-    else
-      let xwin_ids = vimtex#jobs#capture(
-            \ 'xdotool search --name ' . fnamemodify(self.out(), ':t'))
-      let ids_already_used = filter(map(
-            \   deepcopy(vimtex#state#list_all()),
-            \   {_, x -> get(get(x, 'viewer', {}), 'xwin_id')}),
-            \ 'v:val > 0')
-      for id in xwin_ids
-        if index(ids_already_used, id) < 0
-          let self.xwin_id = id
-          break
-        endif
-      endfor
+
+    " First try searching for window ID by viewer PID...
+    let self.xwin_id = self.xdo_find_win_id_by_pid()
+
+    " ...then potentially search by the viewer's window name as a fallback.
+    if self.xwin_id > 0
+      let self.xwin_id = self.xdo_find_win_id_by_name()
     endif
   endif
 

From fa23da3790a25fcf18adfeb0f3e78851b9d08425 Mon Sep 17 00:00:00 2001
From: Elijan Mastnak <ejmastnak@gmail.com>
Date: Mon, 20 Jun 2022 12:00:52 +0200
Subject: [PATCH 3/4] fix: correct unexpected results in xdo_get_id

This commit makes the `xdo_get_id` function attempt to search for a
viewer's X11 window ID in the following order
1. By viewer's PID
2. By viewer's window name
3. By viewer's class

Previously, `xdo_get_id` only attempted a search by viewer class, which
could lead to unexpected results. For details see
[#2415](https://github.com/lervag/vimtex/issues/2415)

refer: [#2415](https://github.com/lervag/vimtex/issues/2415)
---
 autoload/vimtex/view/_template.vim | 33 ++++++++++++++++++------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/autoload/vimtex/view/_template.vim b/autoload/vimtex/view/_template.vim
index 7f3d49f686..f3bc3a07c5 100644
--- a/autoload/vimtex/view/_template.vim
+++ b/autoload/vimtex/view/_template.vim
@@ -136,15 +136,29 @@ function! s:viewer.xdo_get_id() dict abort " {{{1
     " Allow some time for the viewer to start properly
     sleep 500m
 
-    let l:xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
-    if len(l:xwin_ids) == 0
-      call vimtex#log#warning('Viewer cannot find ' . self.name . ' window ID!')
-      let self.xwin_id = 0
-    else
-      let self.xwin_id = l:xwin_ids[-1]
+    " Note: Since `xdo_find_win_id_by_{name,pid,class}()` return zero if no
+    " window ID is found, they can be used to directly set `self.xwin_id`
+    " without introducing and if/else testing intermediate variables like
+    " `l:win_id_by_{name,pid,class}`.
+
+    " First try search by viewer's PID
+    let self.xwin_id = self.xdo_find_win_id_by_pid()
+
+    " If search by PID failed, try search by viewer's window name
+    if self.xwin_id <= 0
+      let self.xwin_id = self.xdo_find_win_id_by_name()
+    endif
+
+    " Fallback to search by viewer class
+    if self.xwin_id <= 0
+      let self.xwin_id = self.xdo_find_win_id_by_class()
     endif
   endif
 
+  if self.xwin_id <= 0
+    call vimtex#log#warning('Viewer cannot find ' . self.name . ' window ID!')
+  endif
+
   return self.xwin_id
 endfunction
 
@@ -176,16 +190,11 @@ function! s:viewer.xdo_exists() dict abort " {{{1
 endfunction
 
 " }}}1
-" Begin new code
-" --------------------------------------------- "
 " Attempts to find the viewer's X window ID using `xdotool` search by window
 " class. Returns the viewer's window ID if one is found, and `0` otherwise.
 function! s:viewer.xdo_find_win_id_by_class() dict abort " {{{1
   let l:xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
   if len(l:xwin_ids) == 0
-    " TODO: probably move logging to the higher-level functions `xdo_exists`
-    " and `xdo_get_id`
-    call vimtex#log#warning('Viewer cannot find ' . self.name . ' window ID!')
     return 0
   else
     return l:xwin_ids[-1]
@@ -229,8 +238,6 @@ function! s:viewer.xdo_find_win_id_by_pid() dict abort " {{{1
 endfunction
 
 " }}}1
-" --------------------------------------------- "
-" End new code
 function! s:viewer.xdo_send_keys(keys) dict abort " {{{1
   if !self.xdo_check() || empty(a:keys) || self.xwin_id <= 0 | return | endif
 

From 0b2a1093850fb6ebdc44f08d8f1088bed79f9491 Mon Sep 17 00:00:00 2001
From: Elijan Mastnak <ejmastnak@gmail.com>
Date: Mon, 20 Jun 2022 13:41:09 +0200
Subject: [PATCH 4/4] refactor: incorporate changes suggested by @lervag

---
 autoload/vimtex/view/_template.vim | 65 +++++++++++-------------------
 1 file changed, 24 insertions(+), 41 deletions(-)

diff --git a/autoload/vimtex/view/_template.vim b/autoload/vimtex/view/_template.vim
index f3bc3a07c5..d2d6f303d1 100644
--- a/autoload/vimtex/view/_template.vim
+++ b/autoload/vimtex/view/_template.vim
@@ -132,28 +132,17 @@ endfunction
 function! s:viewer.xdo_get_id() dict abort " {{{1
   if !self.xdo_check() | return 0 | endif
 
-  if self.xwin_id <= 0
-    " Allow some time for the viewer to start properly
-    sleep 500m
-
-    " Note: Since `xdo_find_win_id_by_{name,pid,class}()` return zero if no
-    " window ID is found, they can be used to directly set `self.xwin_id`
-    " without introducing and if/else testing intermediate variables like
-    " `l:win_id_by_{name,pid,class}`.
+  if self.xwin_id > 0 | return self.xwin_id | endif
 
-    " First try search by viewer's PID
-    let self.xwin_id = self.xdo_find_win_id_by_pid()
+  " Allow some time for the viewer to start properly
+  sleep 500m
 
-    " If search by PID failed, try search by viewer's window name
-    if self.xwin_id <= 0
-      let self.xwin_id = self.xdo_find_win_id_by_name()
-    endif
-
-    " Fallback to search by viewer class
-    if self.xwin_id <= 0
-      let self.xwin_id = self.xdo_find_win_id_by_class()
-    endif
-  endif
+  " Try finding viewer's window ID, first attempting search by PID, then by
+  " window name, and finally, as a fallback, by window class.
+  for l:method in ['pid', 'name', 'class']
+    execute "let self.xwin_id = self.xdo_find_win_id_by_" . l:method . "()"
+    if self.xwin_id > 0 | break | endif
+  endfor
 
   if self.xwin_id <= 0
     call vimtex#log#warning('Viewer cannot find ' . self.name . ' window ID!')
@@ -190,22 +179,18 @@ function! s:viewer.xdo_exists() dict abort " {{{1
 endfunction
 
 " }}}1
-" Attempts to find the viewer's X window ID using `xdotool` search by window
-" class. Returns the viewer's window ID if one is found, and `0` otherwise.
 function! s:viewer.xdo_find_win_id_by_class() dict abort " {{{1
+  " Attempts to find the viewer's X window ID using `xdotool` search by window
+  " class. Returns the viewer's window ID if one is found, and `0` otherwise.
   let l:xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
-  if len(l:xwin_ids) == 0
-    return 0
-  else
-    return l:xwin_ids[-1]
-  endif
+  return empty(l:xwin_ids) ? 0 : l:xwin_ids[-1]
 endfunction
 
 " }}}1
-" Attempts to find the viewer's X window ID using `xdotool` search by window
-" name (i.e. the string in the window titlebar). Returns the viewer's window
-" ID if one is found, and `0` otherwise.
 function! s:viewer.xdo_find_win_id_by_name() dict abort " {{{1
+  " Attempts to find the viewer's X window ID using `xdotool` search by window
+  " name (i.e. the string in the window titlebar). Returns the viewer's window
+  " ID if one is found, and `0` otherwise.
   let l:xwin_ids = vimtex#jobs#capture(
         \ 'xdotool search --name ' . fnamemodify(self.out(), ':t'))
   let ids_already_used = filter(map(
@@ -222,19 +207,17 @@ function! s:viewer.xdo_find_win_id_by_name() dict abort " {{{1
 endfunction
 
 " }}}1
-" Attempts to find the viewer's X window ID using `xdotool` search by the
-" viewer's process ID. Returns the viewer's window ID if one is found, and `0`
-" otherwise.
 function! s:viewer.xdo_find_win_id_by_pid() dict abort " {{{1
+  " Attempts to find the viewer's X window ID using `xdotool` search by the
+  " viewer's process ID. Returns the viewer's window ID if one is found, and `0`
+  " otherwise.
   let l:pid = has_key(self, 'get_pid') ? self.get_pid() : 0
-  if l:pid > 0
-    let xwin_ids = vimtex#jobs#capture(
-          \   'xdotool search --all --pid ' . l:pid
-          \ . ' --name ' . fnamemodify(self.out(), ':t'))
-    return get(xwin_ids, 0)
-  else
-    return 0
-  endif
+  if l:pid <= 0 | return 0 | endif
+
+  let xwin_ids = vimtex#jobs#capture(
+        \   'xdotool search --all --pid ' . l:pid
+        \ . ' --name ' . fnamemodify(self.out(), ':t'))
+  return get(xwin_ids, 0)
 endfunction
 
 " }}}1