Closed guijan closed 4 months ago
The multimonitor code seems to use the explicit display parameter - so removing -D
probably won't buy us much LoC.
I don't know whether this is OK or hacky as I don't use multi-monitor setup.
Also setting an environment variable DISPLAY
is not the same as -D
- the exec-ed child will inherit the new $DISPLAY
but not the one set via -D
(not sure whether this matters in practice or not - but I wanted to make sure we're aware of the subtle changes that this introduces).
I think the difference between -D
and the DISPLAY
environment variable is an unintended consequence more than a feature. Are people really going to run scrot in one X server, and then exec a command in another?
Also, because system()
invokes a shell, the user can make the exec string a script that first calls unset DISPLAY
for this edge case.
I think the difference between
-D
and theDISPLAY
environment variable is an unintended consequence more than a feature. Are people really going to run scrot in one X server, and then exec a command in another?
Same. But as I said, I'm just trying to make sure we fully understand the effect of the change.
Also I looked a bit more into scrotGrabShotMulti
's usage of initXAndImlib
and I don't think we'll gain a whole lot by removing -D
. Unless you want to fix/cleanup scrotGrabShotMulti
(but keep in mind I have no way of testing such changes).
It'll still remove and simplify code if we keep the lines that get the display name from the environment for scrotGrabShotMulti()
. Shouldn't be too hard. We don't have to fix that and deprecate -D
at the same time either.
It'll still remove and simplify code if we keep the lines that get the display name from the environment for
scrotGrabShotMulti()
.
I don't think I follow, what would be removed and simplified exactly? Can you show a patch or something.
Looking at it again, I don't see how removing --display
buys us anything. And it's a straightforward flag to keep supporting, unlike --note
for example.
We don't need to duplicate the work XOpenDisplay() already does for us. Mark --display for removal at a later date.