resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
512 stars 51 forks source link

Add -M/--monitor option to capture a Xinerama display #199

Closed zevweiss closed 1 year ago

zevweiss commented 2 years ago

This is based on the patches in PR #137 by Daniel T. Borelli (daltomi), rebased, tidied up a bit and with a man page update.

Fixes: #38

daltomi commented 2 years ago

Hi @zevweiss It looks good. Before merging with master I would like more contributors to test this PR; one that has 2 or more monitors.

TODO for myself:

N-R-K commented 2 years ago

Since multi-monitor isn't a common case, I think it makes sense to have an option to compile this feature out for those who might not want the extra dependency.

Looking at the code, I don't think it'd require too much ugly ifdefs to achieve. Just guarding the include and the function body should do:

#ifdef ENABLE_MULTIMONITOR
    #include <X11/extensions/Xinerama.h>
#endif

/* ... */

static Imlib_Image scrotGrabShotMonitor(void)
{
#ifdef ENABLE_MULTIMONITOR
    /* ... */
#else
    errx(EXIT_FAILURE, "scrot was compiled without mutli-monitor support");
#endif 
}
zevweiss commented 2 years ago

Since multi-monitor isn't a common case, I think it makes sense to have an option to compile this feature out for those who might not want the extra dependency.

I dunno -- multi-head setups are probably a minority, sure, but I don't think they're a particularly tiny one. Laptop users frequently plug in to an external display in a home/office desk environment or when presenting something via a projector, for example.

Also, while libxinerama is strictly speaking an extra dependency, it's already a dependency of gtk and most desktop environments and window managers (even pretty bare-bones ones like fluxbox/openbox/fvwm, say), so I'd guess the number of systems that have scrot's other dependencies but don't already have it is probably pretty vanishingly small.