pfw: return 1 if no window is focused

[ivzave]

When no window is focused X returns strange window 1 result to pfw which doesn't check it and outputs 0x00000001 returning status 0 in such case. This is confusing and leads to mistakes. My opinion is pfw should output nothing and return status 1 so it can be gracefully checked by caller.

E.g. 'xdotool getwindowfocus' behaves this way.

[Ferdi265]

What X returns here is the special resource POINTER_ROOT (defined as 0x1). This indicates that the window over which the pointer his implicitly gets focus as long as the pointer is there. Depending on some other calls this can also be NONE (defined as 0x0), meaning no window at all has focus.

This should be changed to returning status 1 or similar, as ivzave said.

EDIT: I do think the code should be changed to check against w == XCB_NONE || w == XCB_POINTER_ROOT, to avoid the magic number. This also makes it compatible in case the protocol changes to use other IDs for this.

[z3bra]

Ferdinand Bachmann wrote:

What X returns here is the special resource POINTER_ROOT (defined as 0x1). This indicates that the window over which the pointer his implicitly gets focus as long as the pointer is there. Depending on some other calls this can also be NONE (defined as 0x0), meaning no window at all has focus.

This should be changed to returning status 1 or similar, as ivzave said

It would make sense to use the xcb macros (XCB_NONE / XCB_POINTER_ROOT as said). I'd change the error message to something like "focus not set" (for the sake of pedantism), as in the case of XCB_POINTER_ROOT, there IS a window "focused" (the root window), but all events will be discarded. The current message is explicit enough anyway.

Igor, please update your patch to make use of these two macros.

[Ferdi265]

as in the case of XCB_POINTER_ROOT, there IS a window "focused" (the root window), but all events will be discarded. The current message is explicit enough anyway.

This is not quite true.

When you explicitly focus the root window (wtf $(lsw -r)), you get the ID of the root window, not XCB_POINTER_ROOT.

In fact, XCB_POINTER_ROOT is a special focus case that sends keyboard input to the window under the pointer.

EDIT: from man page XGetInputFocus(3), the Xlib funcion behind xcb_get_input_focus()

   Depending on the focus argument, the following occurs:

   ·    If focus is None, all keyboard events are discarded until a new
        focus window is set, and the revert_to argument is ignored.

   ·    If focus is a window, it becomes the keyboard's focus window.  If
        a generated keyboard event would normally be reported to this
        window or one of its inferiors, the event is reported as usual.
        Otherwise, the event is reported relative to the focus window.

   ·    If focus is PointerRoot, the focus window is dynamically taken to
        be the root window of whatever screen the pointer is on at each
        keyboard event.  In this case, the revert_to argument is ignored.

NOTE: The text if from the section about XSetInputFocus, which resides in the same man page, therefore the talk about a window becoming the focus window

EDIT2: I suggest "focus not set" with exit 1 for XCB_NONE and "dynamic focus" with exit 2 for XCB_POINTER_ROOT

[z3bra]

Ferdinand Bachmann wrote:

as in the case of XCB_POINTER_ROOT, there IS a window "focused" (the root window), but all events will be discarded. The current message is explicit enough anyway.

This is not quite true.

When you explicitly focus the root window (wtf $(lsw -r)), you get the ID of the root window, not XCB_POINTER_ROOT.

In fact, XCB_POINTER_ROOT is a special focus case that sends keyboard input to the window under the pointer.

From /usr/include/xcb/xproto.h:

• If \a focus is XCB_POINTER_ROOT (TODO), focus is on the root window of the
• screen on which the pointer is on currently.

[Ferdi265]

Counter example: run wtf $(lsw -r) && pfw, observe the fact that the return value is NOT in fact XCB_POINTER_ROOT.

These two states are distinct from one another. Also, XCB documentation has been lacking since forever (see the documentation for the enter event, which listed "detail" as TODO), so it could be that whoever wrote it didn't understand the difference.

[z3bra]

Ferdinand Bachmann wrote:

Counter example: run wtf $(lsw -r) && pfw, observe the fact that the return value is NOT in fact XCB_POINTER_ROOT.

These two states are distinct from one another. Also, XCB documentation has been lacking since forever (see the documentation for the enter event, which listed "detail" as TODO), so it could be that whoever wrote it didn't understand the difference.

I think the difference here is that when window is XCB_POINTER_ROOT, the root window is actually focused, but all keybord events are discarded, while events are passed to the root window when it is "explicitely" focused.

Anyway, this debate is sterile, as it won't change anything to how the tool is supposed to behave. Thanks for your input though!

[ivzave]

Updated. XCB_POINTER_ROOT is actually missing from headers but I've found XCB_INPUT_FOCUS_POINTER_ROOT which looks appropriate. See also https://bugs.freedesktop.org/show_bug.cgi?id=89583

[z3bra]

If you want to use different exit codes for different behaviors; you need to edit the manpage and document it. You'll also need to avoid reusing two codes for two different errors (eg, xcb_get_input_focus() failing returns 1 too which might be misleading).

I personally don't see the need for multiple error codes. In the end, all these errors mean the same: the focus is not set to a window. That's all the user really care about, there is no need to make the program more complex than it really is.

[ivzave]

Updated to exit with 1 in case of no window has focus or error. Program is described as 'print focused window' so this should be ok.

[Ferdi265]

I'm happy with this change.

Also: XCB_POINTER_ROOT doesn't mean keyboard events are discarded. They are dynamically sent to wherever the pointer is