[Viewer] Make search for X11 window IDs more robust#2417
Merged
lervag merged 4 commits intolervag:masterfrom Jun 20, 2022
ejmastnak:master
Merged
[Viewer] Make search for X11 window IDs more robust#2417lervag merged 4 commits intolervag:masterfrom ejmastnak:master
lervag merged 4 commits intolervag:masterfrom
ejmastnak:master
Conversation
added 3 commits
June 20, 2022 11:57
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.
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.
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](#2415) refer: [#2415](#2415)
lervag
reviewed
Jun 20, 2022
lervag
reviewed
Jun 20, 2022
lervag
reviewed
Jun 20, 2022
lervag
reviewed
Jun 20, 2022
Owner
|
Thanks! I think this looks very good and is almost ready for merge already. I only had a few minor comments and suggestions. |
Owner
|
Thanks! I made a few adjustments on top, I hope you don't mind. Let me know if I accidentily broke something :p |
Contributor
Author
Looks good! Just tested locally and everything seems to work. Thanks for your suggestions and help implementing this! |
Owner
|
My pleasure; and thanks to you for both the issue and the PR contribution! :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses and solves #2415, wherein the variable
b:vimtex.viewer.xwin_idwould sometimes be set to an X11 window ID that was not the ID of the actual viewer. The issue is solved on my end (Arch Linux 5.18.2 with Zathura), but it would be prudent to test on other systems.The PR is a refactor of existing code more than anything else; here is a quick summary:
xdo_get_idfunction in_template.vimattempt search by PID and search by window name before attempting search by class (previously only search by class was used).More details in the commit messages.
refer: #2415