Hi, * Christian Dietrich <stettberger _at_ dokucode _dot_ de> [2014-07-13 14:41:31 +0200]: > When Xrandr is available at compile and at run time, herbstluftwm > subscribes to the screen change events. These events are send, when > configuration of the monitors changes. The event is propagated to all > herbstclients via the hook. > > randr_screen_change 4108552 1497 724 > > An additional script can listen to those events and can start the > reconfiguration of the herbstluftwm monitors. This is especially useful > for using herbstluftwm within a virtual machine, whose screen size can > change quite often. Do you know about herbstluft's detect_monitors command, and the auto_detect_monitors settings? IMHO it would make more sense to make that work with XRandR (in addition to the existing Xinerama code). Then the other question is if you still need a hook for it, when HLWM reconfigures the monitors automatically. If so, it might make sense to emit it from the Xinerama code as well, if possible. > Hopefully my mail client didn't fuckup the patch. Looks like it did? I have the impression randr.h should be longer than that: > diff --git a/src/randr.h b/src/randr.h > new file mode 100644 > index 0000000..6e26870 > --- /dev/null > +++ b/src/randr.h > @@ -0,0 +1,17 @@ > +/** Copyright 2014 Christian Dietrich. All rights reserved. > + * > + * This software is licensed under the "Simplified BSD License". > + * See LICENSE for details */ > + Then some other feedback and minor nitpicks: > @@ -765,22 +766,24 @@ static HandlerTable g_default_handler = { > static struct { > void (*init)(); > void (*destroy)(); > + bool (*xevent)(XEvent *); > } g_modules[] = { > - { ipc_init, ipc_destroy }, > - { object_tree_init, object_tree_destroy }, > - { key_init, key_destroy }, > - { settings_init, settings_destroy }, > - { floating_init, floating_destroy }, > - { stacklist_init, stacklist_destroy }, > - { layout_init, layout_destroy }, > - { tag_init, tag_destroy }, > - { clientlist_init, clientlist_destroy }, > - { decorations_init, decorations_destroy }, > - { monitor_init, monitor_destroy }, > - { ewmh_init, ewmh_destroy }, > - { mouse_init, mouse_destroy }, > - { hook_init, hook_destroy }, > - { rules_init, rules_destroy }, > + { ipc_init, ipc_destroy , 0}, > + { object_tree_init, object_tree_destroy , 0}, > + { key_init, key_destroy , 0}, > + { settings_init, settings_destroy , 0}, > + { floating_init, floating_destroy , 0}, > + { stacklist_init, stacklist_destroy , 0}, > + { layout_init, layout_destroy , 0}, > + { tag_init, tag_destroy , 0}, > + { clientlist_init, clientlist_destroy , 0}, > + { decorations_init, decorations_destroy , 0}, > + { monitor_init, monitor_destroy , 0}, > + { ewmh_init, ewmh_destroy , 0}, > + { mouse_init, mouse_destroy , 0}, > + { hook_init, hook_destroy , 0}, > + { rules_init, rules_destroy , 0}, > + { randr_init, randr_destroy , randr_xevent}, > }; Wouldn't it make more sense to use NULL instead of 0 here? Also commas are before the spaces in the first column but after the spaces in the second ;) > + /* Query whether the xrandr extension is available in a version >= 1.2 */ > + if (XRRQueryExtension (g_display, &rr_event_base, &ignore)) { > ... For the sake of consistency, you might want to remove the spaces before ( with function calls. I think the current way this is handled in hlwm currently is "foo(...)" but "while (...)" (i.e., spaces only with control structures). > + HSDebug("randr: xrandr version is too old (%d.%d); no screen notify events will be availabe", > + major_version, minor_version); s/availabe/available/ ;) > + HSDebug("randr: xrandr extension is not available"); There's a newline missing (same with some other HSDebug calls). > + char timestamp[30] , width[30], height[30]; No space before that comma :P Florian -- http://www.the-compiler.org | me _at_ the _minus_ compiler _dot_ org (Mail/XMPP) GPG 0xFD55A072 | http://the-compiler.org/pubkey.asc I love long mails! | http://email.is-not-s.ms/
Attachment:
pgpjGrkGlppfs.pgp
Description: PGP signature