Add window title component

[chrisduerr]

This component displays the title of the currently focused window.

See issue #3.

[dogamak]

This shouldn't probably be done by polling, you can get notifications of EWMH property changes from XCB, but we don't have currently any way for components to receive these. Not at least from the bar window.

The component could create it's own connection to Xorg and create an invisible window and listen for the notifications from that.

[chrisduerr]

Oh so there is some kind of event listener system for xcb? Do you know how I can create such an event listener? And I'd assume that your issue is with the fixed poll every X seconds, right?

[chrisduerr]

    let (conn, _) = Connection::connect(None).unwrap();
    loop {
        let event = conn.wait_for_event();
    }

I ran this simple event listener in the stream method but it just locks up and doesn't receive any event. Not entirely sure how this is supposed to work.

It seems like it requires a window but that would kinda defeat the purpose. After all I don't want any event that affects a certain window.

[chrisduerr]

I think I found a way to get all property change events. Which does cover this but also covers a few more things. It's better than a fixed timer tho. However I'm not quite sure how to make this into a Stream/Future.

[dogamak]

There's XcbEventStream in src/xcb_event_stream.rs, which you can use to wrap xcb::Connection and use as a stream, it doesn't block.

[chrisduerr]

Oh it's neat that there already is a simple wrapper for this in the code, I didn't see it. But I had that much figured out already, my problem is just that I can't share conn across threads. So I can't box it.

    fn stream(self, _: Handle) -> Self::Stream {
        let (conn, screen_num) = Connection::connect(None).unwrap();
        let setup = conn.get_setup();
        let screen = setup.roots().nth(screen_num as usize).unwrap();

        xcb::change_window_attributes(
            &conn,
            screen.root(),
            &[(xcb::CW_EVENT_MASK, xcb::EVENT_MASK_PROPERTY_CHANGE)],
        );

        conn.flush();
        xcb_event_stream::XcbEventStream::new(conn)
            .and_then(move |_| Ok(get_window_title()))
            .boxed()

    }

This is what I have right now. Unfortunately I have no clue of how tokio works.

[dogamak]

You could maybe create the connection in the main thread using handle.spawn(closure) and return a channel receiver.

    fn stream(self, handle: Handle) -> Self::Stream {
        let (tx, rx) = channel();
        handle.spawn(|| {
            let (conn, screen_num) = Connection::connect(None).unwrap();
            xcb_event_stream::XcbEventStream::new(conn)
                .map(|_| get_window_title())
                .forward(tx)
        });

        Box::new(rx)
    }

The above is pseudopseudocode. Also, wow, github comment textbox sucks for code editing.

[chrisduerr]

I'm not sure how I would make that work. I don't think I'll be able to make this work with my current knowledge of tokio.

[dogamak]

Yeah, it might get tricky. But considering we might want to implement the workspace component too, the property change events should probably be provided to any component needing them.

[chrisduerr]

I'm trying to make it work somehow. It compiles but unfortunately doesn't do stuff. I'll add to this PR if I find out how to do it.

[chrisduerr]

@dogamak I figured out how to solve this. It was a stupid issue with .boxed() and Box::new. Now this should properly work with futures. I have tested it and it seems to work properly. Except for the issues that I have fixed in #32.

[chrisduerr]

On a sidenote, this still updates more often than really required. However it's not a big difference. I'm not sure how to change that right now tho.

[dogamak]

WindowTitle should propably be an unit struct, just to make configuration cleaner.

I can't get this to work at all. I tried to modify this to use _NET_WM_NAME without any luck, although it doesn't seem to be receiving any event's at all.

[dogamak]

Oops

[dogamak]

It should be possible to check the property that has changed by casting the GenericEvent to PropertyNotifyEvent using cast_event and comparing the atom field of that to the atom representing _NET_WM_NAME.

[chrisduerr]

I'll try out casting the event but I'm not sure what I can do about it not working for you. It works just fine on my machine.

Especially with _NET_WM_NAME it should work. Which WM are you using?

[dogamak]

I am using bspwm, but I tested it in gnome as well with the same results. You are using i3, right?

[chrisduerr]

Yes that is correct. If you're using xprop to inspect windows, are the properties set?

[chrisduerr]

All unnecessary events should now get filtered. One issue left is that the title is blank unless an event is set, even when a window is in focus. But I'm not quite sure how important that issue is.

I'd love to figure out the problems you're having tho @dogamak. Can you show me the _NET_WM_NAME version you tried out?

[chrisduerr]

Uhm interesting. I'm not getting the error Travis is having locally.

[chrisduerr]

I'm just seeing that the Cargo.lock is inside the .gitignore. I would recommend changing that to make sure builds are always working and using the same versions of libraries.

[dogamak]

Libraries should not include Cargo.lock in the repository, as stated in the crates.io FAQ. The .gitignore in this repo is the default one created by cargo.

[dogamak]

Actually, now that I think about it again, I have no idea how this could work even for you. To get the title of the active window one should first fetch _NET_ACTIVE_WINDOW of the root window and then get the _NET_WM_NAME of the window referenced by that.

So now this should always return the title of the bar window, but it isn't doing even that. Not at least for me.

[chrisduerr]

Oh wow I totally forgot for a second that xcbars is a library…

Yeah then no cargo.lock is completely correct then.

[chrisduerr]

I don't use _NET_ACTIVE_WINDOW because I'm using xcb::get_input_focus to get the currently focused window. I thought that might work regardless of EWMH support. Then I just get the xcb::ATOM_WM_NAME of that window. Which is just a constant for _WM_NAME.

So all you should have to do is create a _NET_WM_NAME atom and then replace the xcb::ATOM_WM_NAME.

[chrisduerr]

@dogamak I pushed a commit that uses _NET_WM_NAME and works for me too. Because i3 seems to support both. If you could try this out it would help a lot. If this doesn't work, I have no idea why.

[dogamak]

Still doesn't work for me. xprop shows that _NET_ACTIVE_WINDOW and _NET_WM_NAME are being set properly, so using them directly would probably be better than using xcb::get_input_focus. I can't find any documentation on what it actually does, so it might be using some obsolete or less adopted used standard.

Unfortunately I cannot figure out the format xcb is using for window ids. Xcb returns [9, 0, 32, 3] as a value for _NET_ACTIVE_WINDOW when xprop shows the value as 0x3200009. Deserializing it as little endian or big endian u32 doesn't work.

[chrisduerr]

That's super odd. I'd expect the implemented xcb methods to work on every WM since they should be part of the standard protocol. It works with both _NET_WM_NAME and WM_NAME for me tho. So it can only really be the xcb::get_input_focus method that fails.

With _NET_ACTIVE_WINDOW (if I can make it work) I'll have to support both the default and the ewmh way again tho… Why can't these people pick one thing and stick to it.

[chrisduerr]

@dogamak Would you mind replacing your get_window_title with this: https://gist.github.com/chrisduerr/788268e324fd321617f46b1f7ebb0127

This is using _NET_ACTIVE_WINDOW and works on my machine. If this works for you I could try to implement it properly to support both. If this doesn't work… I have no idea what the problem is.

[dogamak]

I found the problem. It's your filtering, at least partially. With the function from that gist and after replacing const ATOM_FOCUS_CHANGE with xcb::intern_atom(&conn, true, "_NET_ACTIVE_WINDOW") the component works properly.

[chrisduerr]

What? That can't be possible! ATOM_FOCUS_CHANGE is the _NET_ACTIVE_WINDOW atom as a constant…

It's 334. Are the atoms different for different systems?

Also turns out that this would never have worked for systems without _NET_ACTIVE_WINDOW. Because I filtered for ATOM_FOCUS_CHANGE which is _NET_ACTIVE_WINDOW. I would probably need a completely different event for WMs that do not support EWMH.

[chrisduerr]

@dogamak I now acquire the atoms when first setting up the WindowTitle. Try if this works, it should work on every WM that supports EWMH.

How important do you think it is to support WMs that do not implement EWMH? Should I add that again?

[dogamak]

Seems to work now. Initialization, which is currently done in WindowTitle::new, should preferably be done in init method of the Component trait, while the initial WindowTitle struct should be created using Default. Ideally, we could keep the configuration type as an unit struct but the current Component trait isn't quite flexible enough. Other than that everything's looking pretty good.

[chrisduerr]

@dogamak I've reworked it again. If you have any more comments, please let me know.

[dogamak]

I think this PR is now fine, however I noticed a slight delay between switching windows and the title updating in the bar. This led me to discover that I needed to go few abstraction levels deeper to get a proper integration between XCB and tokio.

I have reimplemented XcbEventStream using mio in #35, so if you'd rebase this to use the new implementation (after I manage to merge that) I'd be happy to merge this.

[chrisduerr]

Well those commits got a little mixed up but I guess it works? The change in responsibility is quite dramatic, it's a lot smoother now.

[dogamak]

Seems to be working for both me and Travis, so I'll merge this now.

Thanks for your work.

[chrisduerr]

I tried. :D