multi-monitor support #1
Merged
Conversation
This file contains 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
Added support for multiple monitors. While sxiv defaults to full-screen, this is limited to the current monitor. Adding `-Z` ensures that if you're on the top-left monitor, this works as expected. Monitors will open full-screen views of the screenshot with the top left pixel of the _monitor_ showing the top left pixel of the screenshot, which is only appropriate on the top-left-most monitor. I also used `trap` to ensure `$tmpfile` is cleaned up regardless of whether the script is terminated. Nice observations on speed! `maim` is a _lot_ faster than ImageMagick's `import` and `sxiv` is quite speedy as well. The script will be slightly faster using `/bin/sh` over `/bin/bash` (which would be slightly faster than `/usr/bin/env bash`), and since there are no bashisms, the more basic POSIX shell is functionally identical.
|
These are some really good remarks, things that I haven't thought about. One thing though, have you checked if this script is working without any error? |
|
Yeah, you caught me having not tested my code. I'm not sure how the CHLD signal got in there, it triggered on each child process termination (including itself‽), which was not desired. I've fixed that and actually tested. I also added |
|
Thank you @adamhotep for this change. |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Added (slightly better) support for multiple monitors. When
sxivgoes full-screen, it is on a per-monitor basis. Adding-Zensures that if you're on the top-left monitor, this works as expected. Monitors will open full-screen views of the screenshot with the top left pixel of the monitor showing the top left pixel of the screenshot, so you'll see the wrong content on any other monitor.I also used
trapto ensure$tmpfileis cleaned up regardless of whether the script is terminated.Nice observations on speed!
maimis a lot faster than ImageMagick'simportandsxivis quite speedy as well. The script will be slightly faster using/bin/shover/bin/bash(which would be slightly faster than/usr/bin/env bash), and since there are no bashisms, the more basic POSIX shell is functionally identical.