Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Holding onto destroyed client Xids in a state extension breaks NamedScratchpad behaviour #268

Closed
2 tasks done
sminez opened this issue Jun 22, 2023 · 5 comments · Fixed by #269
Closed
2 tasks done

Holding onto destroyed client Xids in a state extension breaks NamedScratchpad behaviour #268

sminez opened this issue Jun 22, 2023 · 5 comments · Fixed by #269
Assignees
@sminez
Labels
bug Something isn't working

Comments

@sminez
Copy link
Owner

sminez commented Jun 22, 2023 •
edited
Loading

Originally reported by @greweln on Discord
Originally titled "The NamedScratchpad extensions sometimes takes ownership of the wrong window", renamed following discovery of the cause of the bug so that it is easier to find in future.

Describe the bug

https://github.com/sminez/penrose/blob/develop/src/extensions/hooks/named_scratchpads.rs

When running the KeyEventHandler for a NamedScratchpad when there is currently no managed scratchpad window and an existing window is present on the workspace running the same program, the scratchpad takes ownership of the existing window rather than correctly spawning a new instance. Triggering the KeyEventHandler again brings the window back as a scratchpad and it will now act as the scratchpad window until it is closed. This happens despite the Query being used by the scratchpad not matching the window in question.

So far this has only been observed with st so more testing is needed to confirm that this is a general issue rather than an issue with st under Penrose specifically.

The only place where we should be setting the client id for a named scratchpad is in the manage hook and that is guarded by the provided Query matching the newly managed window.

The behaviour seen as the tiled window has been pulled into the scratchpad looks to be the first branch of the following if statement (moving the client to the NSP tag as it is currently on-screen):

        if state.client_set.current_workspace().contains(&id) {
            // Toggle off: hiding the client on our invisible workspace
            debug!("current workspace contains target client: moving to NSP tag");
            state.client_set.move_client_to_tag(&id, NSP_TAG);
        } else {
            // Toggle on / bring to current workspace
            debug!("current workspace does not contain target client: moving to tag");
            state.client_set.move_client_to_current_tag(&id);

            if self.run_hook_on_toggle {
                if let Err(e) = hook.call(id, state, x) {
                    error!(%e, %name, "unable to run NSP manage hook during toggle");
                }
            }
        }

To Reproduce

Example WM from @greweln exhibiting the behaviour: https://github.com/greweln/gwm/tree/main
I've also been able to reproduce this on my current window manager: https://github.com/sminez/my-penrose-config

Steps to reproduce the behavior:

Run the minimal main.rs shown below, then run the following:

  • M-/ (toggle scratchpad)
  • M-S-q (kill scratchpad)
  • M-Return (spawn tiled terminal)
  • M-/ (toggle scratchpad) <-- triggers the bug

It looks as though you can't reporoduce without killing the inital scratchpad window?

Expected behavior

Existing tiled windows are not moved into NamedScratchpads.

Additional context

  • minimal main.rs that reproduces the issue
use penrose::{
    builtin::actions::{exit, modify_with, spawn},
    core::{bindings::parse_keybindings_with_xmodmap, WindowManager},
    extensions::hooks::{add_named_scratchpads, manage::FloatingCentered, NamedScratchPad},
    map,
    x::query::ClassName,
    x11rb::RustConn,
    Result,
};
use std::collections::HashMap;
use tracing_subscriber::{self, prelude::*};

fn main() -> Result<()> {
    tracing_subscriber::fmt()
        .with_env_filter("trace")
        .finish()
        .init();

    let (nsp, toggle_scratch) = NamedScratchPad::new(
        "terminal",
        "st -c StScratchpad",
        ClassName("StScratchpad"),
        FloatingCentered::new(0.8, 0.8),
        true,
    );

    let mut raw_bindings = map! {
        map_keys: |k: &str| k.to_string();

        "M-S-q" => modify_with(|cs| cs.kill_focused()),
        "M-slash" => Box::new(toggle_scratch),
        "M-Return" => spawn("st"),
        "M-A-Escape" => exit(),
    };

    for tag in &["1", "2", "3", "4", "5", "6", "7", "8", "9"] {
        raw_bindings.extend([(format!("M-{tag}"), modify_with(move |cs| cs.focus_tag(tag)))]);
    }

    let conn = RustConn::new()?;
    let key_bindings = parse_keybindings_with_xmodmap(raw_bindings)?;
    let wm = add_named_scratchpads(
        WindowManager::new(Default::default(), key_bindings, HashMap::new(), conn)?,
        vec![nsp],
    );

    wm.run()
}
  • debug log output
    The following was provided by @greweln
2023-06-20T21:59:24.426840Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 8, code: 53 })
2023-06-20T21:59:24.426924Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-20T21:59:24.426940Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 8, code: 53 }
2023-06-20T21:59:24.426964Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: spawning NamedScratchPad program nsp.prog=st -c Exit_Menu -e exit_menu name=exit_menu
2023-06-20T21:59:24.445817Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(18874373))
2023-06-20T21:59:24.445833Z TRACE XEvent{event=MapRequest}: penrose::core: running user event hook
2023-06-20T21:59:24.445840Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(18874373)
2023-06-20T21:59:24.445869Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(18874373)
2023-06-20T21:59:24.445873Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=18874373
2023-06-20T21:59:24.445878Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=18874373
2023-06-20T21:59:24.445897Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=18874373
2023-06-20T21:59:24.445929Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=18874373
2023-06-20T21:59:24.445946Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-20T21:59:24.445995Z DEBUG XEvent{event=MapRequest}: penrose::extensions::hooks::named_scratchpads: matched query for named scratchpad scratchpad="exit_menu" id=18874373
2023-06-20T21:59:24.446016Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(18874373)
2023-06-20T21:59:24.446058Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=18874373 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 14, h: 24 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 804, h: 484 }) }
2023-06-20T21:59:24.446067Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=18874373 r=Rect { x: 672, y: 378, w: 572, h: 320 }
2023-06-20T21:59:24.446072Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(18874373)
2023-06-20T21:59:24.446099Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-20T21:59:24.446103Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-20T21:59:24.446106Z TRACE XEvent{event=MapRequest}: penrose::x: running user refresh hook

...

2023-06-20T22:05:51.085714Z TRACE XEvent{event=UnmapNotify}: penrose::core: event details details=UnmapNotify(Xid(18874373))
2023-06-20T22:05:51.085766Z TRACE XEvent{event=UnmapNotify}: penrose::core: running user event hook
2023-06-20T22:05:51.085778Z TRACE XEvent{event=UnmapNotify}: penrose::x: removing client client=Xid(18874373)
2023-06-20T22:05:51.085802Z TRACE XEvent{event=UnmapNotify}: penrose::x: hiding client c=Xid(18874373)
2023-06-20T22:05:51.085814Z TRACE XEvent{event=UnmapNotify}: penrose::x: setting withdrawn state for client c=Xid(18874373)
2023-06-20T22:05:51.085832Z TRACE XEvent{event=UnmapNotify}: penrose::x: checking if focus should change
2023-06-20T22:05:51.085835Z TRACE XEvent{event=UnmapNotify}: penrose::x: running user refresh hook
2023-06-20T22:05:51.086148Z TRACE XEvent{event=UnmapNotify}: penrose::core: event details details=UnmapNotify(Xid(18874373))
2023-06-20T22:05:51.086154Z TRACE XEvent{event=UnmapNotify}: penrose::core: running user event hook
2023-06-20T22:05:51.086163Z TRACE XEvent{event=Leave}: penrose::core: event details details=Leave(PointerChange { id: Xid(18874373), abs: Point { x: 960, y: 540 }, relative: Point { x: 286, y: 160 }, same_screen: false })
2023-06-20T22:05:51.086168Z TRACE XEvent{event=Leave}: penrose::core: running user event hook
2023-06-20T22:05:51.086172Z TRACE XEvent{event=Destroy}: penrose::core: event details details=Destroy(Xid(18874373))
2023-06-20T22:05:51.086174Z TRACE XEvent{event=Destroy}: penrose::core: running user event hook
2023-06-20T22:05:51.086177Z TRACE XEvent{event=Destroy}: penrose::core::handle: destroying client client=Xid(18874373)
2023-06-20T22:05:51.086180Z TRACE XEvent{event=Destroy}: penrose::x: removing client client=Xid(18874373)
2023-06-20T22:05:51.086186Z TRACE XEvent{event=Destroy}: penrose::x: checking if focus should change
2023-06-20T22:05:51.086188Z TRACE XEvent{event=Destroy}: penrose::x: running user refresh hook
2023-06-20T22:05:51.086345Z TRACE XEvent{event=Destroy}: penrose::core: event details details=Destroy(Xid(18874373))
2023-06-20T22:05:51.086350Z TRACE XEvent{event=Destroy}: penrose::core: running user event hook
2023-06-20T22:05:51.086353Z TRACE XEvent{event=Destroy}: penrose::core::handle: destroying client client=Xid(18874373)
2023-06-20T22:05:51.086355Z TRACE XEvent{event=Destroy}: penrose::x: removing client client=Xid(18874373)
2023-06-20T22:05:51.086361Z TRACE XEvent{event=Destroy}: penrose::x: checking if focus should change
2023-06-20T22:05:51.086363Z TRACE XEvent{event=Destroy}: penrose::x: running user refresh hook
2023-06-20T22:05:51.086823Z ERROR penrose::core: Error pulling next x event e=X11 error: X11Error { error_kind: Window, error_code: 3, sequence: 1044, bad_value: 18874373, minor_opcode: 0, major_opcode: 2, extension_name: None, request_name: Some("ChangeWindowAttributes") }
2023-06-20T22:05:51.086844Z ERROR penrose::core: Error pulling next x event e=X11 error: X11Error { error_kind: Window, error_code: 3, sequence: 1045, bad_value: 18874373, minor_opcode: 0, major_opcode: 2, extension_name: None, request_name: Some("ChangeWindowAttributes") }
2023-06-20T22:05:51.086847Z ERROR penrose::core: Error pulling next x event e=X11 error: X11Error { error_kind: Window, error_code: 3, sequence: 1046, bad_value: 18874373, minor_opcode: 0, major_opcode: 10, extension_name: None, request_name: Some("UnmapWindow") }
2023-06-20T22:05:51.086850Z ERROR penrose::core: Error pulling next x event e=X11 error: X11Error { error_kind: Window, error_code: 3, sequence: 1047, bad_value: 18874373, minor_opcode: 0, major_opcode: 2, extension_name: None, request_name: Some("ChangeWindowAttributes") }
2023-06-20T22:05:51.086852Z ERROR penrose::core: Error pulling next x event e=X11 error: X11Error { error_kind: Window, error_code: 3, sequence: 1048, bad_value: 18874373, minor_opcode: 0, major_opcode: 18, extension_name: None, request_name: Some("ChangeProperty") }
2023-06-20T22:05:51.086855Z ERROR penrose::core: Error pulling next x event e=X11 error: X11Error { error_kind: Window, error_code: 3, sequence: 1049, bad_value: 18874373, minor_opcode: 0, major_opcode: 18, extension_name: None, request_name: Some("ChangeProperty") }

...

2023-06-20T22:05:53.867639Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 8, code: 36 })
2023-06-20T22:05:53.867692Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-20T22:05:53.867707Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 8, code: 36 }
2023-06-20T22:05:53.886071Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(18874373))
2023-06-20T22:05:53.886089Z TRACE XEvent{event=MapRequest}: penrose::core: running user event hook
2023-06-20T22:05:53.886096Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(18874373)
2023-06-20T22:05:53.886132Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(18874373)
2023-06-20T22:05:53.886137Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=18874373
2023-06-20T22:05:53.886142Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=18874373
2023-06-20T22:05:53.886162Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=18874373
2023-06-20T22:05:53.886195Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=18874373
2023-06-20T22:05:53.886211Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-20T22:05:53.886254Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(18874373)
2023-06-20T22:05:53.886296Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=18874373 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 14, h: 24 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 804, h: 484 }) }
2023-06-20T22:05:53.886305Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=18874373 r=Rect { x: 10, y: 38, w: 1896, h: 1028 }
2023-06-20T22:05:53.886310Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(18874373)
2023-06-20T22:05:53.886318Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-20T22:05:53.886323Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-20T22:05:53.886330Z TRACE XEvent{event=MapRequest}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true
2023-06-20T22:05:53.886892Z TRACE XEvent{event=MapRequest}: penrose::x: running user refresh hook

...

2023-06-20T22:10:58.533234Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 8, code: 53 })
2023-06-20T22:10:58.533278Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-20T22:10:58.533292Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 8, code: 53 }
2023-06-20T22:10:58.533313Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: Toggling nsp client current_tag="1" current_screen=0
2023-06-20T22:10:58.533338Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: current workspace contains target client: moving to NSP tag
2023-06-20T22:10:58.533348Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: running refresh
2023-06-20T22:10:58.533395Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: setting border for focused client focused=Xid(20971564)
2023-06-20T22:10:58.533987Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: client has WmNormalHints: applying size hints client=20971564 hints=WmNormalHints { flags: P_MIN_SIZE | P_MAX_SIZE | P_BASE_SIZE | P_WIN_GRAVITY, base: Some(Rect { x: 0, y: 0, w: 450, h: 120 }), min: Some(Rect { x: 0, y: 0, w: 450, h: 120 }), max: Some(Rect { x: 0, y: 0, w: 16384, h: 16384 }), user_specified: None }
2023-06-20T22:10:58.534044Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: positioning client client=20971564 r=Rect { x: 10, y: 38, w: 1896, h: 1028 }
2023-06-20T22:10:58.534064Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: revealing client c=Xid(20971564)
2023-06-20T22:10:58.534080Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: hiding client c=Xid(18874373)
2023-06-20T22:10:58.534117Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: checking if focus should change
2023-06-20T22:10:58.534126Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: focused client changed
2023-06-20T22:10:58.534135Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true
2023-06-20T22:10:58.535341Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: running user refresh hook

...

2023-06-20T22:10:58.536353Z TRACE XEvent{event=UnmapNotify}: penrose::core: event details details=UnmapNotify(Xid(18874373))
2023-06-20T22:10:58.536357Z TRACE XEvent{event=UnmapNotify}: penrose::core: running user event hook

...

2023-06-20T22:11:11.782602Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 8, code: 53 })
2023-06-20T22:11:11.782666Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-20T22:11:11.782705Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 8, code: 53 }
2023-06-20T22:11:11.782731Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: Toggling nsp client current_tag="2" current_screen=0
2023-06-20T22:11:11.782749Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: current workspace does not contain target client: moving to tag
2023-06-20T22:11:11.782773Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: running refresh
2023-06-20T22:11:11.782814Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: setting border for focused client focused=Xid(18874373)
2023-06-20T22:11:11.783467Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: client has WmNormalHints: applying size hints client=25165829 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 14, h: 24 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 804, h: 484 }) }
2023-06-20T22:11:11.783507Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: positioning client client=25165829 r=Rect { x: 10, y: 38, w: 1896, h: 1028 }
2023-06-20T22:11:11.783637Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: client has WmNormalHints: applying size hints client=18874373 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 14, h: 24 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 804, h: 484 }) }
2023-06-20T22:11:11.783671Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: positioning client client=18874373 r=Rect { x: 672, y: 378, w: 572, h: 320 }
2023-06-20T22:11:11.783684Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: revealing client c=Xid(25165829)
2023-06-20T22:11:11.783711Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: revealing client c=Xid(18874373)
2023-06-20T22:11:11.783729Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: checking if focus should change
2023-06-20T22:11:11.783738Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: focused client changed
2023-06-20T22:11:11.783747Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: running user refresh hook

...

2023-06-20T22:11:14.027198Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 8, code: 53 })
2023-06-20T22:11:14.027286Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-20T22:11:14.027302Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 8, code: 53 }
2023-06-20T22:11:14.027338Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: Toggling nsp client current_tag="2" current_screen=0
2023-06-20T22:11:14.027358Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: current workspace contains target client: moving to NSP tag
2023-06-20T22:11:14.027373Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: running refresh
2023-06-20T22:11:14.027422Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: setting border for focused client focused=Xid(25165829)
2023-06-20T22:11:14.028132Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: client has WmNormalHints: applying size hints client=25165829 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 14, h: 24 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 804, h: 484 }) }
2023-06-20T22:11:14.028201Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: positioning client client=25165829 r=Rect { x: 10, y: 38, w: 1896, h: 1028 }
2023-06-20T22:11:14.028249Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: revealing client c=Xid(25165829)
2023-06-20T22:11:14.028267Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: hiding client c=Xid(18874373)
2023-06-20T22:11:14.028296Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: checking if focus should change
2023-06-20T22:11:14.028310Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: focused client changed
2023-06-20T22:11:14.028319Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: warping to focused client focus_changed=true focused_client_moved=false
2023-06-20T22:11:14.029127Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "exit_menu", run_hook_on_toggle: true }}: penrose::x: running user refresh hook
@sminez sminez added the bug Something isn't working label Jun 22, 2023
@sminez sminez self-assigned this Jun 22, 2023
sminez added a commit that referenced this issue Jun 22, 2023
@sminez
#268 adding more logging around NamedScratchpad behaviour
1f63180
@sminez
Copy link
Owner Author

sminez commented Jun 22, 2023

The following is a log obtained from the minimal reproduction steps outlined above. If you pay attention to the client IDs it looks as though the same ID ends up being used again for the tiled window as the scratchpad window which was just closed?

### Startup
2023-06-22T20:46:18.955808Z TRACE penrose::core::bindings: parsed keybinding pattern="M-S-q" mask=65 code=24
2023-06-22T20:46:18.955850Z TRACE penrose::core::bindings: parsed keybinding pattern="M-5" mask=64 code=14
2023-06-22T20:46:18.955856Z TRACE penrose::core::bindings: parsed keybinding pattern="M-6" mask=64 code=15
2023-06-22T20:46:18.955860Z TRACE penrose::core::bindings: parsed keybinding pattern="M-8" mask=64 code=17
2023-06-22T20:46:18.955865Z TRACE penrose::core::bindings: parsed keybinding pattern="M-1" mask=64 code=10
2023-06-22T20:46:18.955869Z TRACE penrose::core::bindings: parsed keybinding pattern="M-9" mask=64 code=18
2023-06-22T20:46:18.955874Z TRACE penrose::core::bindings: parsed keybinding pattern="M-Return" mask=64 code=36
2023-06-22T20:46:18.955878Z TRACE penrose::core::bindings: parsed keybinding pattern="M-slash" mask=64 code=61
2023-06-22T20:46:18.955884Z TRACE penrose::core::bindings: parsed keybinding pattern="M-A-Escape" mask=72 code=9
2023-06-22T20:46:18.955888Z TRACE penrose::core::bindings: parsed keybinding pattern="M-2" mask=64 code=11
2023-06-22T20:46:18.955893Z TRACE penrose::core::bindings: parsed keybinding pattern="M-3" mask=64 code=12
2023-06-22T20:46:18.955897Z TRACE penrose::core::bindings: parsed keybinding pattern="M-4" mask=64 code=13
2023-06-22T20:46:18.955901Z TRACE penrose::core::bindings: parsed keybinding pattern="M-7" mask=64 code=16
2023-06-22T20:46:19.023936Z  INFO penrose::core: registering SIGCHILD signal handler
2023-06-22T20:46:19.023981Z TRACE penrose::core::handle: grabbing key and mouse bindings
2023-06-22T20:46:19.024086Z  INFO manage_existing_clients: penrose::core: managing existing clients
2023-06-22T20:46:19.024366Z  INFO manage_existing_clients: penrose::core: triggering refresh
2023-06-22T20:46:19.024415Z TRACE manage_existing_clients: penrose::x: checking if focus should change

### Toggle scratchpad (spawns the initial terminal)
2023-06-22T20:46:21.855893Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 64, code: 61 })
2023-06-22T20:46:21.855954Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 64, code: 61 }
2023-06-22T20:46:21.855992Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: spawning NamedScratchPad program nsp.prog=st -c StScratchpad name=terminal nsp.client=None
2023-06-22T20:46:21.894074Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(8388613))
2023-06-22T20:46:21.894109Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(8388613)
2023-06-22T20:46:21.894149Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(8388613)
2023-06-22T20:46:21.894157Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=8388613
2023-06-22T20:46:21.894162Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=8388613
2023-06-22T20:46:21.894193Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=8388613
2023-06-22T20:46:21.894252Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=8388613
2023-06-22T20:46:21.894284Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-22T20:46:21.894334Z DEBUG XEvent{event=MapRequest}: penrose::extensions::hooks::named_scratchpads: matched query for named scratchpad scratchpad="terminal" id=8388613
2023-06-22T20:46:21.894362Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(8388613)
2023-06-22T20:46:21.894419Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-22T20:46:21.894429Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=8388613 r=Rect { x: 137, y: 77, w: 1088, h: 610 }
2023-06-22T20:46:21.894436Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(8388613)
2023-06-22T20:46:21.894447Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-22T20:46:21.894451Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-22T20:46:21.894454Z TRACE XEvent{event=MapRequest}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true
2023-06-22T20:46:21.894898Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 0, y: 0, w: 640, h: 432 }, is_root: false })
2023-06-22T20:46:21.894909Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 0, y: 0, w: 640, h: 432 }, is_root: false })
2023-06-22T20:46:21.894922Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 137, y: 77, w: 1088, h: 610 }, is_root: false })
2023-06-22T20:46:21.894928Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 137, y: 77, w: 1088, h: 610 }, is_root: false })
2023-06-22T20:46:21.894938Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })
2023-06-22T20:46:21.894950Z TRACE XEvent{event=Enter}: penrose::core: event details details=Enter(PointerChange { id: Xid(8388613), abs: Point { x: 683, y: 384 }, relative: Point { x: 544, y: 305 }, same_screen: false })
2023-06-22T20:46:21.894966Z TRACE XEvent{event=Enter}: penrose::x: setting border for focused client focused=Xid(8388613)
2023-06-22T20:46:21.895259Z TRACE XEvent{event=Enter}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-22T20:46:21.895303Z TRACE XEvent{event=Enter}: penrose::x: positioning client client=8388613 r=Rect { x: 137, y: 77, w: 1088, h: 610 }
2023-06-22T20:46:21.895337Z TRACE XEvent{event=Enter}: penrose::x: revealing client c=Xid(8388613)
2023-06-22T20:46:21.895383Z TRACE XEvent{event=Enter}: penrose::x: checking if focus should change
2023-06-22T20:46:21.895628Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })
2023-06-22T20:46:21.899389Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_HINTS", is_root: false })
2023-06-22T20:46:21.942629Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_HINTS", is_root: false })
2023-06-22T20:46:22.041346Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_NAME", is_root: false })
2023-06-22T20:46:22.041412Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "_NET_WM_NAME", is_root: false })

### Close initial scratchpad
2023-06-22T20:46:22.378434Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 65, code: 24 })
2023-06-22T20:46:22.378503Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 65, code: 24 }
2023-06-22T20:46:22.378831Z TRACE XEvent{event=KeyPress}: penrose::x: hiding client c=Xid(8388613)
2023-06-22T20:46:22.378907Z TRACE XEvent{event=KeyPress}: penrose::x: setting withdrawn state for client c=Xid(8388613)
2023-06-22T20:46:22.378958Z TRACE XEvent{event=KeyPress}: penrose::x: checking if focus should change
2023-06-22T20:46:22.379350Z TRACE XEvent{event=UnmapNotify}: penrose::core: event details details=UnmapNotify(Xid(8388613))
2023-06-22T20:46:22.379425Z TRACE XEvent{event=Leave}: penrose::core: event details details=Leave(PointerChange { id: Xid(8388613), abs: Point { x: 683, y: 384 }, relative: Point { x: 544, y: 305 }, same_screen: false })
2023-06-22T20:46:22.380515Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })
2023-06-22T20:46:22.380569Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })
2023-06-22T20:46:22.383230Z TRACE XEvent{event=Destroy}: penrose::core: event details details=Destroy(Xid(8388613))
2023-06-22T20:46:22.383295Z TRACE XEvent{event=Destroy}: penrose::core::handle: destroying client client=Xid(8388613)
2023-06-22T20:46:22.383324Z TRACE XEvent{event=Destroy}: penrose::x: removing client client=Xid(8388613)
2023-06-22T20:46:22.383380Z TRACE XEvent{event=Destroy}: penrose::x: checking if focus should change
2023-06-22T20:46:22.383464Z TRACE XEvent{event=Destroy}: penrose::core: event details details=Destroy(Xid(8388613))
2023-06-22T20:46:22.383495Z TRACE XEvent{event=Destroy}: penrose::core::handle: destroying client client=Xid(8388613)
2023-06-22T20:46:22.383519Z TRACE XEvent{event=Destroy}: penrose::x: removing client client=Xid(8388613)
2023-06-22T20:46:22.383560Z TRACE XEvent{event=Destroy}: penrose::x: checking if focus should change

### Spawn tiled terminal
2023-06-22T20:46:22.940857Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 64, code: 36 })
2023-06-22T20:46:22.940943Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 64, code: 36 }
2023-06-22T20:46:22.979218Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(8388613))
2023-06-22T20:46:22.979283Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(8388613)
2023-06-22T20:46:22.979326Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(8388613)
2023-06-22T20:46:22.979335Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=8388613
2023-06-22T20:46:22.979340Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=8388613
2023-06-22T20:46:22.979372Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=8388613
2023-06-22T20:46:22.979432Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=8388613
2023-06-22T20:46:22.979465Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-22T20:46:22.979485Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(8388613)
2023-06-22T20:46:22.979552Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-22T20:46:22.979563Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=8388613 r=Rect { x: 0, y: 0, w: 1362, h: 764 }
2023-06-22T20:46:22.979571Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(8388613)
2023-06-22T20:46:22.979581Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-22T20:46:22.979585Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-22T20:46:22.979589Z TRACE XEvent{event=MapRequest}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true
2023-06-22T20:46:22.979949Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 0, y: 0, w: 640, h: 432 }, is_root: false })
2023-06-22T20:46:22.979969Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 0, y: 0, w: 640, h: 432 }, is_root: false })
2023-06-22T20:46:22.979992Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 0, y: 0, w: 1362, h: 764 }, is_root: false })
2023-06-22T20:46:22.980003Z TRACE XEvent{event=ConfigureNotify}: penrose::core: event details details=ConfigureNotify(ConfigureEvent { id: Xid(8388613), r: Rect { x: 0, y: 0, w: 1362, h: 764 }, is_root: false })
2023-06-22T20:46:22.980014Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })
2023-06-22T20:46:22.980026Z TRACE XEvent{event=Enter}: penrose::core: event details details=Enter(PointerChange { id: Xid(8388613), abs: Point { x: 683, y: 384 }, relative: Point { x: 681, y: 382 }, same_screen: false })
2023-06-22T20:46:22.980045Z TRACE XEvent{event=Enter}: penrose::x: setting border for focused client focused=Xid(8388613)
2023-06-22T20:46:22.980386Z TRACE XEvent{event=Enter}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-22T20:46:22.980448Z TRACE XEvent{event=Enter}: penrose::x: positioning client client=8388613 r=Rect { x: 0, y: 0, w: 1362, h: 764 }
2023-06-22T20:46:22.980469Z TRACE XEvent{event=Enter}: penrose::x: revealing client c=Xid(8388613)
2023-06-22T20:46:22.980493Z TRACE XEvent{event=Enter}: penrose::x: checking if focus should change
2023-06-22T20:46:22.980634Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })
2023-06-22T20:46:22.986146Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_HINTS", is_root: false })
2023-06-22T20:46:23.035606Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_HINTS", is_root: false })
2023-06-22T20:46:23.128163Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_NAME", is_root: false })
2023-06-22T20:46:23.128200Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "_NET_WM_NAME", is_root: false })

### Toggle scratchpad (grabs the tiled terminal)
2023-06-22T20:46:23.684084Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 64, code: 61 })
2023-06-22T20:46:23.684146Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 64, code: 61 }
2023-06-22T20:46:23.684183Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: NamedScratchPad client exists in state id=8388613 name=terminal
2023-06-22T20:46:23.684206Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: Toggling nsp client id=8388613 name=terminal current_tag="1" current_screen=0
2023-06-22T20:46:23.684229Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: current workspace contains target client: moving to NSP tag id=8388613
2023-06-22T20:46:23.684248Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: running refresh following NamedScratchPad toggle id=8388613 name=terminal
2023-06-22T20:46:23.684297Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::x: hiding client c=Xid(8388613)
2023-06-22T20:46:23.684344Z TRACE XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::x: checking if focus should change
2023-06-22T20:46:23.684646Z TRACE XEvent{event=UnmapNotify}: penrose::core: event details details=UnmapNotify(Xid(8388613))
2023-06-22T20:46:23.684735Z TRACE XEvent{event=Leave}: penrose::core: event details details=Leave(PointerChange { id: Xid(8388613), abs: Point { x: 683, y: 384 }, relative: Point { x: 681, y: 382 }, same_screen: false })
2023-06-22T20:46:23.685523Z TRACE XEvent{event=PropertyNotify}: penrose::core: event details details=PropertyNotify(PropertyEvent { id: Xid(8388613), atom: "WM_STATE", is_root: false })

@greweln
Copy link

greweln commented Jun 23, 2023

I played around with the minimal config you set up above:

  • I can confirm after adding my scratchpads to the config that the behavior is the one you described:
  1. toggle scratchpad

  2. kill the spawned scratchpad terminal

  3. spawn a tiled terminal

  4. toggle again
  • I noticed as well that the window which becomes scratchpad gets the same ID as the previous closed scratchpad. I'm not sure if this the normal behavior.
  • This behavior seems to be related to St only. I ran a test spawning the scratchpads in Alacritty.... everything works well.

sminez added a commit that referenced this issue Jun 23, 2023
@sminez
Fixes #268: explicitly clearing NSP state on client destroyed
e2d0fda 
So far it has only been observed with the st terminal, but it looks like it is possible
for a newly spawned window to re-use the Xid of a previous client of the same program.
If a NamedScratchpad still thinks it owns a window with that ID it will run the toggle
logic under the assumption that the window in question is the one it owned previously.

The initial fix for this is to add an event hook that checks for NamedScratchpad clients
being destroyed and explicitly removes the stored ID from state. Alternatively we could
also look at running the NSP Query every time the toggle binding is run but this feels
like a better approach as it keeps the toggle logic as simple as possible.
@sminez
Copy link
Owner Author

sminez commented Jun 23, 2023

Yeah I've not been able to reproduce the id being reused with a program other than st, it's probably worth looking into why that's happening and if it is expected behaviour that I should be handling.

The above commit fixes the issue for me by explicitly removing closed scratchpad clients from the State extension used by NamedScratchpad. @greweln when you have time can you take a look at running the reproduction steps again using this updated branch and see if it fixes the issue for you as well? If so I think we can merge the fix and close this one off 👍

This is a log of a fresh restart of the minimal main.rs with the following actions taken:

  • M-/ (toggle scratchpad)
  • M-S-q (kill scratchpad)
  • M-Return (spawn tiled terminal)
  • M-/ (toggle scratchpad)

The fix has been to add an event hook that explicitly clears the state in the NamedScratchPad state extension
when a scratchpad client has been destroyed.

NOTE: For readability, the ConfigureNotify, PropertyNotify, Enter and Leave event logs have been omitted

### Startup
2023-06-23T05:24:34.385724Z TRACE penrose::core::bindings: parsed keybinding pattern="M-3" mask=64 code=12
2023-06-23T05:24:34.385773Z TRACE penrose::core::bindings: parsed keybinding pattern="M-A-Escape" mask=72 code=9
2023-06-23T05:24:34.385781Z TRACE penrose::core::bindings: parsed keybinding pattern="M-slash" mask=64 code=61
2023-06-23T05:24:34.385787Z TRACE penrose::core::bindings: parsed keybinding pattern="M-7" mask=64 code=16
2023-06-23T05:24:34.385793Z TRACE penrose::core::bindings: parsed keybinding pattern="M-2" mask=64 code=11
2023-06-23T05:24:34.385799Z TRACE penrose::core::bindings: parsed keybinding pattern="M-8" mask=64 code=17
2023-06-23T05:24:34.385804Z TRACE penrose::core::bindings: parsed keybinding pattern="M-1" mask=64 code=10
2023-06-23T05:24:34.385810Z TRACE penrose::core::bindings: parsed keybinding pattern="M-Return" mask=64 code=36
2023-06-23T05:24:34.385816Z TRACE penrose::core::bindings: parsed keybinding pattern="M-4" mask=64 code=13
2023-06-23T05:24:34.385822Z TRACE penrose::core::bindings: parsed keybinding pattern="M-5" mask=64 code=14
2023-06-23T05:24:34.385827Z TRACE penrose::core::bindings: parsed keybinding pattern="M-S-q" mask=65 code=24
2023-06-23T05:24:34.385832Z TRACE penrose::core::bindings: parsed keybinding pattern="M-9" mask=64 code=18
2023-06-23T05:24:34.385838Z TRACE penrose::core::bindings: parsed keybinding pattern="M-6" mask=64 code=15
2023-06-23T05:24:34.478978Z  INFO penrose::core: registering SIGCHILD signal handler
2023-06-23T05:24:34.479029Z TRACE penrose::core::handle: grabbing key and mouse bindings
2023-06-23T05:24:34.479158Z  INFO manage_existing_clients: penrose::core: managing existing clients
2023-06-23T05:24:34.479381Z  INFO manage_existing_clients: penrose::core: triggering refresh
2023-06-23T05:24:34.479428Z TRACE manage_existing_clients: penrose::x: checking if focus should change

### Toggle scratchpad (spawns the initial terminal)
2023-06-23T05:24:39.816782Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 64, code: 61 })
2023-06-23T05:24:39.816849Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-23T05:24:39.816869Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 64, code: 61 }
2023-06-23T05:24:39.816913Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: spawning NamedScratchPad program nsp.prog=st -c StScratchpad name=terminal nsp.client=None
2023-06-23T05:24:39.863055Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(8388613))
2023-06-23T05:24:39.863079Z TRACE XEvent{event=MapRequest}: penrose::core: running user event hook
2023-06-23T05:24:39.863085Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(8388613)
2023-06-23T05:24:39.863119Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(8388613)
2023-06-23T05:24:39.863125Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=8388613
2023-06-23T05:24:39.863130Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=8388613
2023-06-23T05:24:39.863157Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=8388613
2023-06-23T05:24:39.863204Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=8388613
2023-06-23T05:24:39.863230Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-23T05:24:39.863274Z DEBUG XEvent{event=MapRequest}: penrose::extensions::hooks::named_scratchpads: matched query for named scratchpad scratchpad="terminal" id=8388613
2023-06-23T05:24:39.863300Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(8388613)
2023-06-23T05:24:39.863363Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-23T05:24:39.863374Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=8388613 r=Rect { x: 137, y: 77, w: 1088, h: 610 }
2023-06-23T05:24:39.863381Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(8388613)
2023-06-23T05:24:39.863391Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-23T05:24:39.863395Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-23T05:24:39.863399Z TRACE XEvent{event=MapRequest}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true
...

### Close initial scratchpad
> The highlighted line in this trace is what fixes the bug

2023-06-23T05:24:40.820955Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 65, code: 24 })
2023-06-23T05:24:40.821025Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-23T05:24:40.821044Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 65, code: 24 }
2023-06-23T05:24:40.821374Z TRACE XEvent{event=KeyPress}: penrose::x: hiding client c=Xid(8388613)
2023-06-23T05:24:40.821464Z TRACE XEvent{event=KeyPress}: penrose::x: setting withdrawn state for client c=Xid(8388613)
2023-06-23T05:24:40.821525Z TRACE XEvent{event=KeyPress}: penrose::x: checking if focus should change
2023-06-23T05:24:40.821850Z TRACE XEvent{event=UnmapNotify}: penrose::core: event details details=UnmapNotify(Xid(8388613))
2023-06-23T05:24:40.821951Z TRACE XEvent{event=UnmapNotify}: penrose::core: running user event hook
...
2023-06-23T05:24:40.825951Z TRACE XEvent{event=Destroy}: penrose::core: event details details=Destroy(Xid(8388613))
2023-06-23T05:24:40.825979Z TRACE XEvent{event=Destroy}: penrose::core: running user event hook
`2023-06-23T05:24:40.826002Z DEBUG XEvent{event=Destroy}: penrose::extensions::hooks::named_scratchpads: scratchpad client destroyed sp.name=terminal destroyed=8388613`
2023-06-23T05:24:40.826032Z TRACE XEvent{event=Destroy}: penrose::core::handle: destroying client client=Xid(8388613)
2023-06-23T05:24:40.826055Z TRACE XEvent{event=Destroy}: penrose::x: removing client client=Xid(8388613)
2023-06-23T05:24:40.826110Z TRACE XEvent{event=Destroy}: penrose::x: checking if focus should change
2023-06-23T05:24:40.826178Z TRACE XEvent{event=Destroy}: penrose::core: event details details=Destroy(Xid(8388613))
2023-06-23T05:24:40.826199Z TRACE XEvent{event=Destroy}: penrose::core: running user event hook
2023-06-23T05:24:40.826212Z TRACE XEvent{event=Destroy}: penrose::core::handle: destroying client client=Xid(8388613)
2023-06-23T05:24:40.826226Z TRACE XEvent{event=Destroy}: penrose::x: removing client client=Xid(8388613)
2023-06-23T05:24:40.826253Z TRACE XEvent{event=Destroy}: penrose::x: checking if focus should change


### Spawn tiled terminal
> It looks like this newly spawned terminal still has the same Xid as the destroyed scratchpad for me?
> I need to read up on whether or not that is expected behaviour.

2023-06-23T05:24:41.824988Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 64, code: 36 })
2023-06-23T05:24:41.825047Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-23T05:24:41.825102Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 64, code: 36 }
2023-06-23T05:24:41.860063Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(8388613))
2023-06-23T05:24:41.860086Z TRACE XEvent{event=MapRequest}: penrose::core: running user event hook
2023-06-23T05:24:41.860092Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(8388613)
2023-06-23T05:24:41.860132Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(8388613)
2023-06-23T05:24:41.860140Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=8388613
2023-06-23T05:24:41.860145Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=8388613
2023-06-23T05:24:41.860174Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=8388613
2023-06-23T05:24:41.860219Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=8388613
2023-06-23T05:24:41.860242Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-23T05:24:41.860293Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(8388613)
2023-06-23T05:24:41.860352Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-23T05:24:41.860361Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=8388613 r=Rect { x: 0, y: 0, w: 1362, h: 764 }
2023-06-23T05:24:41.860368Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(8388613)
2023-06-23T05:24:41.860378Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-23T05:24:41.860382Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-23T05:24:41.860385Z TRACE XEvent{event=MapRequest}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true
...


### Toggle scratchpad (now correctly spawns a new terminal)
> See highlighted line for the corrected scratchpad behaviour of spawning the new terminal
> rather than hiding the tiled window

2023-06-23T05:24:42.850072Z TRACE XEvent{event=KeyPress}: penrose::core: event details details=KeyPress(KeyCode { mask: 64, code: 61 })
2023-06-23T05:24:42.850140Z TRACE XEvent{event=KeyPress}: penrose::core: running user event hook
2023-06-23T05:24:42.850161Z TRACE XEvent{event=KeyPress}: penrose::core::handle: running user keybinding key=KeyCode { mask: 64, code: 61 }
`2023-06-23T05:24:42.850196Z DEBUG XEvent{event=KeyPress}:call{self=ToggleNamedScratchPad { name: "terminal", run_hook_on_toggle: true }}: penrose::extensions::hooks::named_scratchpads: spawning NamedScratchPad program nsp.prog=st -c StScratchpad name=terminal nsp.client=None`

2023-06-23T05:24:42.897811Z TRACE XEvent{event=MapRequest}: penrose::core: event details details=MapRequest(Xid(10485765))
2023-06-23T05:24:42.897836Z TRACE XEvent{event=MapRequest}: penrose::core: running user event hook
2023-06-23T05:24:42.897841Z TRACE XEvent{event=MapRequest}: penrose::core::handle: handling new map request client=Xid(10485765)
2023-06-23T05:24:42.897883Z TRACE XEvent{event=MapRequest}: penrose::core::handle: managing client client=Xid(10485765)
2023-06-23T05:24:42.897900Z TRACE XEvent{event=MapRequest}: penrose::x: managing new client id=10485765
2023-06-23T05:24:42.897906Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmTransientFor prop id=10485765
2023-06-23T05:24:42.897940Z TRACE XEvent{event=MapRequest}: penrose::x: fetching WmClass prop client=10485765
2023-06-23T05:24:42.898002Z TRACE XEvent{event=MapRequest}: penrose::x: fetching NetWmWindowType prop client=10485765
2023-06-23T05:24:42.898036Z TRACE XEvent{event=MapRequest}: penrose::x: running user manage hook
2023-06-23T05:24:42.898091Z DEBUG XEvent{event=MapRequest}: penrose::extensions::hooks::named_scratchpads: matched query for named scratchpad scratchpad="terminal" id=10485765
2023-06-23T05:24:42.898120Z TRACE XEvent{event=MapRequest}: penrose::x: setting border for focused client focused=Xid(10485765)
2023-06-23T05:24:42.898278Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=8388613 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-23T05:24:42.898291Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=8388613 r=Rect { x: 0, y: 0, w: 1362, h: 764 }
2023-06-23T05:24:42.898355Z TRACE XEvent{event=MapRequest}: penrose::x: client has WmNormalHints: applying size hints client=10485765 hints=WmNormalHints { flags: P_SIZE | P_MIN_SIZE | P_RESIZE_INC | P_BASE_SIZE, base: Some(Rect { x: 0, y: 0, w: 4, h: 4 }), min: Some(Rect { x: 0, y: 0, w: 12, h: 22 }), max: None, user_specified: Some(Rect { x: 0, y: 0, w: 640, h: 432 }) }
2023-06-23T05:24:42.898364Z TRACE XEvent{event=MapRequest}: penrose::x: positioning client client=10485765 r=Rect { x: 137, y: 77, w: 1088, h: 610 }
2023-06-23T05:24:42.898371Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(8388613)
2023-06-23T05:24:42.898380Z TRACE XEvent{event=MapRequest}: penrose::x: revealing client c=Xid(10485765)
2023-06-23T05:24:42.898392Z TRACE XEvent{event=MapRequest}: penrose::x: checking if focus should change
2023-06-23T05:24:42.898397Z TRACE XEvent{event=MapRequest}: penrose::x: focused client changed
2023-06-23T05:24:42.898401Z TRACE XEvent{event=MapRequest}: penrose::x: warping to focused client focus_changed=true focused_client_moved=true

@sminez
Copy link
Owner Author

sminez commented Jun 23, 2023

Now that we know the issue is around Xids being re-used I've down a bit more digging and found a fair amount of bug reports for different programs relating to this:

I think it's safe to say that hanging on to Xids in a state extension without checking for Destroy events is going to lead to sadness. I might need look into adding a trait for state extensions that lets me notify them of destroyed Xids? One to look into later I suspect!

@sminez sminez linked a pull request Jun 23, 2023 that will close this issue
268 clearing destroyed Xids from NSP state #269
Merged
@sminez sminez mentioned this issue Jun 23, 2023
Merged
@sminez sminez changed the title The NamedScratchpad extensions sometimes takes ownership of the wrong window Holding onto destroyed client Xids in a state extension breaks NamedScratchpad behaviour Jun 23, 2023
@greweln
Copy link

greweln commented Jun 23, 2023

Woot! looks good now. Thanks!

sminez added a commit that referenced this issue Jun 23, 2023
@sminez
268 clearing destroyed Xids from NSP state (#269)
c26566b 
* #268 adding more logging around NamedScratchpad behaviour

* Fixes #268: explicitly clearing NSP state on client destroyed

So far it has only been observed with the st terminal, but it looks like it is possible
for a newly spawned window to re-use the Xid of a previous client of the same program.
If a NamedScratchpad still thinks it owns a window with that ID it will run the toggle
logic under the assumption that the window in question is the one it owned previously.

The initial fix for this is to add an event hook that checks for NamedScratchpad clients
being destroyed and explicitly removes the stored ID from state. Alternatively we could
also look at running the NSP Query every time the toggle binding is run but this feels
like a better approach as it keeps the toggle logic as simple as possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sminez sminez

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

268 clearing destroyed Xids from NSP state sminez/penrose
2 participants
@sminez @greweln