cycle_client no longer works in a hook

[crypt17]

Describe the bug
between now and yesterday calling the cycle_client function from a remove client hook seems to have become a no op

To Reproduce
Steps to reproduce the behavior:
with the hook in place open multiple windows and then close them.

The expected behaviour is that the focus would change in the given direction but at present the focus remains the same regardless of the value passed to cycle_client. Neither Forward or backward make any change.

Additional context
This is the hook that I am using. Even if it is reduced to just the cycle_client command it has no effect now bt was working yesterday 15/9

struct MyHook {}
impl Hook for MyHook {
    fn remove_client(&mut self, _wm: &mut WindowManager, _id: WinId) {
        // check if we need to do something
        // if not bail else move down
        let ws = match _wm.workspace(&Selector::Focused) {
            Some(ws) => ws,
            None => return 
        };

        let the_len =  ws.len();
        
        if the_len < 2 { return; }

        let cref = _wm.client(&Selector::Focused).unwrap();
        let end_id = match _wm.client(&Selector::Index(the_len-1)) {
            Some(c) => c.id(),
            None => return
        };


         // check if this is the last item
         if cref.id() == end_id {
             return;
         }

        // swap down

        _wm.cycle_client(Direction::Backward);
    }
}

[sminez]

Are you able to give me the hash that this was last working on? It would also really help if you can put some logging into your hook and determine exactly when it is exiting. Looking over the code you have provided there are quite a few places where you could be exiting early.

[crypt17]

sorry not sure what the hash was. I think that is was going before I put in the issue with the workspace cycling code. I put in debug code and it was exiting at the end of the block after the cycle client call. I also stripped it down to just the cycle_client call and it appeared not to be doing anything.

…

On Wed, 16 Sep 2020, 17:49 Innes Anderson-Morrison, < ***@***.***> wrote: Are you able to give me the hash that this was last working on? It would also really help if you can put some logging into your hook and determine exactly when it is exiting. Looking over the code you have provided there are quite a few places where you could be exiting early. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#65 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJN5S4Z6Z2ZFUZF56LIQF3LSGBGYJANCNFSM4ROAMEOQ> .

[sminez]

@crypt17 I've rewritten your hook a bit to make it easier to reason about and added some simple print logging to trace the execution:

struct RmClientHook {}
impl Hook for RmClientHook {
    fn remove_client(&mut self, wm: &mut WindowManager, _: WinId) {
        if let Some(ws) = wm.workspace(&Selector::Focused) {
            println!("have focused workspace: {}", ws.name());
            if ws.len() > 1 {
                println!("more than one client (n={})", ws.len());
                let last = wm.client(&Selector::Index(ws.len() - 1));
                if last != wm.client(&Selector::Focused) {
                    println!("new focused is not last");
                    wm.cycle_client(Backward);
                }
            }
        }
    }
}

From what I can see, this is firing (yours was as well) and cycle_client is being called. cycle_client is also doing the right thing. When using a layout that does not trigger on a focus change it works fine but it results in an infinite loop of cycling clients for the example paper layout config. Working around that is something that you'll need to handle in your hook if this is how you are wanting to implement it.

[crypt17]

I am back in xmonad at the moment but will have a play with this when I am next back looking at penrose.

…

On Sat, 19 Sep 2020, 21:13 Innes Anderson-Morrison, < ***@***.***> wrote: Closed #65 <#65>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#65 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJN5S46Y3D7N5A37XJGEBX3SGRY4XANCNFSM4ROAMEOQ> .

[crypt17]

after thinking about it I think that some of this code should be in core rather than as a plugin to mirror the way that there are the options that can be set for new client.

…

On Sun, 20 Sep 2020, 17:53 joe skinner, ***@***.***> wrote: I am back in xmonad at the moment but will have a play with this when I am next back looking at penrose. On Sat, 19 Sep 2020, 21:13 Innes Anderson-Morrison, < ***@***.***> wrote: > Closed #65 <#65>. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#65 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AJN5S46Y3D7N5A37XJGEBX3SGRY4XANCNFSM4ROAMEOQ> > . >

[sminez]

This behaviour is a breaking change for layouts that follow focus. It will not be added to core. If you want to implement it as am extension as already discussed then feel free to do so.