bl-kb does not handle Keybinds wih multiple actions

[thierrybo]

Hi,

If you create an action in rc.xml like this:

    <keybind key="W-Left">
      <action name="UnmaximizeFull"/>
      <action name="MaximizeVert"/>
      <action name="MoveResizeTo">
        <width>50%</width>
        <x>0</x>
        <y>0</y>
      </action>
    </keybind>

bl-kb will not handle it correctly and it will not appear in Keybindings list. It seems that it is any Keybinding with more that one action that cause this issue.

Regards,

TB

[johnraff]

@thierrybo thank you for reporting this.

I have just confirmed on my own system, although in my case the keybinding is displayed, but with only the first defined action. Example:

    <keybind key="W-Left">
      <action name="ToggleMaximize"><direction>vertical</direction></action>
      <action name="Raise"/>
      <action name="MoveResizeTo">
        <width>50%</width>
        <x>0</x>
        <y>0</y>
      </action>
    </keybind>

Anyway, we must see if there's an easy fix for this.

[ghost]

The bug is very simple. https://github.com/BunsenLabs/bunsen-utilities/blob/helium/bin/bl-kb#L82 drops all but the first <action> node among the children but it needs to handle the case of multiple <action>s inside a <keybind>.

[johnraff]

Assigned the author @capn-damo to this issue.

[ghost]

@johnraff @capn-damo

I've rewritten bl-kb to take multiple actions in account (porting it to Python 3 at the same time). Could you perhaps test bl-kb from the branch wip-bl-kb-rewrite of this repository? I'm a bit unsure if bl-kb-pipemenu handles the spaces in the rightmost column when aggregating actions as described below.

• Output to yad now looks like this (in yad, the alignment with tabs is correct). Actions of the same type (that is, in bl-kb-submenu lingo, unclickable o-actions and clickable x-actions are now aggregated on a per-key basis: if the key defines multiple commands to execute, commands are aggregated by appending them to each other using the ; separator which should allow executing them when run though a shell; and if multpiple actions (o) are defined, the action names are concatenated using ,

  o   C-A-Left            GoToDesktop
  o   C-A-Right           GoToDesktop
  o   C-A-Up              GoToDesktop
  o   C-A-Down            GoToDesktop
  o   S-A-Left            SendToDesktop
  o   S-A-Right           SendToDesktop
  o   S-A-Up              SendToDesktop
  o   S-A-Down            SendToDesktop
  o   W-F1                GoToDesktop
  o   W-F2                GoToDesktop
  o   W-F3                GoToDesktop
  o   W-F4                GoToDesktop
  o   W-d                 ToggleShowDesktop
  o   A-F4                Close
  o   A-F5                Iconify
  o   A-F6                ToggleMaximize
  o   A-Escape            Lower, FocusToBottom, Unfocus
  o   A-space             client-menu
  x   Print               xfce4-screenshooter
  x   A-Print             scrot '%Y-%m-%d--%s_$wx$h_scrot.png' -e 'mv $f ~/Pictures/screenshots/ ; bl-image-viewer ~/Pictures/screenshots/$f'
  o   A-Tab               NextWindow
  o   A-S-Tab             PreviousWindow
  o   C-A-Tab             NextWindow
  o   W-S-Right           DirectionalCycleWindows
  o   W-S-Left            DirectionalCycleWindows
  o   W-S-Up              DirectionalCycleWindows
  o   W-S-Down            DirectionalCycleWindows
  x   A-F2                gmrun
  x   A-F3                ~/.config/dmenu/dmenu-bind.sh
  x   W-f                 bl-file-manager
  x   W-t                 x-terminal-emulator
  x   W-w                 x-www-browser
  x   W-l                 bl-lock
  x   W-e                 bl-text-editor
  x   W-m                 bl-media-player
  x   W-h                 x-terminal-emulator -T 'htop task manager' -e htop
  x   W-v                 pavucontrol
  x   W-x                 bl-exit
  x   W-Escape            xkill
  o   W-Tab               client-list-combined-menu
  o   W-space             root-menu
  o   A-C-q               root-menu
  o   W-Right             GrowToEdgeEast
  o   W-Left              GrowToEdgeWest
  o   W-Down              GrowToEdgeSouth
  o   W-Up                GrowToEdgeNorth
  o   W-A-Left            UnmaximizeFull, MaximizeVert, MoveResizeTo
  o   W-A-Right           UnmaximizeFull, MaximizeVert, MoveResizeTo
  o   A-r                 Resize
  o   A-m                 Move

• Output to bl-kb-pipemenu looks like this (with less tabs)

  o C-A-Left              GoToDesktop
  o C-A-Right             GoToDesktop
  o C-A-Up                GoToDesktop
  o C-A-Down              GoToDesktop
  o S-A-Left              SendToDesktop
  o S-A-Right             SendToDesktop
  o S-A-Up                SendToDesktop
  o S-A-Down              SendToDesktop
  o W-F1                  GoToDesktop
  o W-F2                  GoToDesktop
  o W-F3                  GoToDesktop
  o W-F4                  GoToDesktop
  o W-d                   ToggleShowDesktop
  o A-F4                  Close
  o A-F5                  Iconify
  o A-F6                  ToggleMaximize
  o A-Escape              Lower, FocusToBottom, Unfocus
  o A-space               client-menu
  x Print                 xfce4-screenshooter
  x A-Print               scrot '%Y-%m-%d--%s_$wx$h_scrot.png' -e 'mv $f ~/Pictures/screenshots/ ; bl-image-viewer ~/Pictures/screenshots/$f'
  o A-Tab                 NextWindow
  o A-S-Tab               PreviousWindow
  o C-A-Tab               NextWindow
  o W-S-Right             DirectionalCycleWindows
  o W-S-Left              DirectionalCycleWindows
  o W-S-Up                DirectionalCycleWindows
  o W-S-Down              DirectionalCycleWindows
  x A-F2                  gmrun
  x A-F3                  ~/.config/dmenu/dmenu-bind.sh
  x W-f                   bl-file-manager
  x W-t                   x-terminal-emulator
  x W-w                   x-www-browser
  x W-l                   bl-lock
  x W-e                   bl-text-editor
  x W-m                   bl-media-player
  x W-h                   x-terminal-emulator -T 'htop task manager' -e htop
  x W-v                   pavucontrol
  x W-x                   bl-exit
  x W-Escape              xkill
  o W-Tab                 client-list-combined-menu
  o W-space               root-menu
  o A-C-q                 root-menu
  o W-Right               GrowToEdgeEast
  o W-Left                GrowToEdgeWest
  o W-Down                GrowToEdgeSouth
  o W-Up                  GrowToEdgeNorth
  o W-A-Left              UnmaximizeFull, MaximizeVert, MoveResizeTo
  o W-A-Right             UnmaximizeFull, MaximizeVert, MoveResizeTo
  o A-r                   Resize
  o A-m                   Move

[ghost]

To add: I've so far also excluded this change by damo from a while back: https://github.com/BunsenLabs/bunsen-utilities/commit/83b4d195e9784d94cad5c577c233aba6e7d2fce6#diff-2c4ff6649c36fff457c276471576ed78 since because auf action aggregation as implemented now, the output speaks the 'truth' about all configured keybindings, and because linked change would only prevent UnmaximizeFull to appear, not MaximizeVert and so on. If you Openbox users like to get all tiling actions and suchlike excluded, then it can be done easily. Just let me know your preference.

[johnraff]

"Display keybinds in new window" correctly outputs the contents of my rc.xml to yad, similarly to your example. ie bl-kb --gui seems to be OK.

However, there are issues with the --keybinds option of bl-kb-pipemenu, ie bl-kb with no option.

1) Biggest, it overwrites rc.xml with the same keybinds content.

2) The output menu, when displayed, still fails to show multiple actions, or anything at all for W-A-left. The keybinds.txt file which is generated does not contain the correct data.

OK, now found line 165 should reference kbinds.txt, not rc.xml:

    ap.add_argument("--openbox-kbinds",
            default="{}/.config/openbox/kbinds.txt".format(os.getenv("HOME", "/root")),
            help="Output file to save the keybind list to")

But even fixing that, multiple actions or the W-A-left binding are still not shown. kbinds.txt is now OK so emit_textfile() and emit_gui() are working. There might be a problem with keybind2lines()?

[ghost]

Biggest, it overwrites rc.xml with the same keybinds content.

That was a mistake in the option parser; fixed. openbox-kbinds pointed to the wrong path (the RC file (I'm not using the defaults)).

When you run bl-kb alone (either --gui or without) it should show the correct setup. I think the output format is fine this way; bl-kb-pipemenu is probably choking on https://github.com/BunsenLabs/bunsen-pipemenus/blob/helium/bin/bl-kb-pipemenu#L84 and https://github.com/BunsenLabs/bunsen-pipemenus/blob/helium/bin/bl-kb-pipemenu#L93 where it accepts only exeactly three fields while bl-kb outputs nice-to-read keybind listings for your 'problematic' key like so

o W-A-Left              UnmaximizeFull, MaximizeVert, MoveResizeTo

which would be hard to read if we removed the space between the , and the text. Maybe quoting does work (seems to work with the x actions (executable actions)). I need to get OB running to test for myself it looks like.

[johnraff]

Biggest, it overwrites rc.xml with the same keybinds content.

That was a mistake in the option parser; fixed. openbox-kbinds pointed to the wrong path (the RC file (I'm not using the defaults)).

Where have you fixed this? Line 165 is still wrong, I think: https://github.com/BunsenLabs/bunsen-utilities/blob/wip-bl-kb-rewrite/bin/bl-kb#L165 Anyway, if that line is fixed, then bl-kb does its job OK - the problem is with bl-kb-pipemenu.

The generated xml in fact looks fine in a terminal, so it must be an Openbox escaping issue.

        <item label="W-A-Left             UnmaximizeFull, MaximizeVert, MoveResizeTo">
            <action name="Execute">
                <command>
                    echo &gt;/dev/null 2&gt;&amp;1
                </command>
            </action>
        </item>

Anyway, the outputted menu is not as neat as the code clearly means it to be. Openbox is changing the spacing, and dropping everything after the first comma. This can be confirmed by hardcoding a string on line 86 of bl-kb-pipemenu.

[ghost]

Forgot to push.

For the rest (the alignment) I need to install BL myself. I don't see why a comma would need escaping; it's not a magic XML character. Perhaps it is the label length? I'll see.

[johnraff]

Apologies - the menu is now OK - thanks! Halfway through my testing I forgot to check between jgmenu and openbox's native rendering.

The comma issue is with jgmenu's xml parsing, and I'll pass it over to @johanmalm

The alignment issue is simply the proportional font. If Openbox is set to use a monospace font for menu items then everything lines up nicely. I doubt if there's anything more we can do about that. EDIT: Adding the encoded tab character 	 does improve alignment with proportional fonts, but cannot be passed through our xml escape function because the ampersand will become literal. Probably too much trouble. Meanwhile xml:space="preserve" seems not to be supported.

I've just confirmed the latest bl-kb copied from wip-bl-kb-rewrite results in a working keybinds menu. Would you like to merge it into helium and close this issue?

[ghost]

@johnraff Thank you for double-checking. I squashed the branch and opened a pull requeset (#64).

I'm always open to improving the menu, but if you don't know a trick how to improve the pipemenu layout short of using a proportional font (ideally, we'd have something like 'expanding tabs/spacers' to put between key and action description, like LaTeX's \hfill) it solves the current issue and therefore is merge-ready.

[johanmalm]

Nice rewrite @2ion

Openbox doesn't appear to use pango-markup. Otherwise we might have been able to do a "pango table" or similar to tabulate.

Anyway, I've fixed the shortcoming in jgmenu 2f6e6eba

[johanmalm]

^ Ignore last, I misread. "Pango tables" is something different.

I've played around a bit with pango_layout_set_tabs() and jgmenu. I think we could easily tidy it up to look like this...

[ghost]

@johanmalm ^ Looking good (no idea how it looked before, I've done this blind).

Let me know if I have to change something, and if yes what I have to change in the output of bl-kb to make things work smoothly at your end.

[johanmalm]

@2ion bl-kb shouldn’t need to change. I think I’ll just replace a group of spaces with a tab in the ob module.

Sent from my iPhone

On 21 Nov 2018, at 23:28, Jens John notifications@github.com wrote:

@johanmalm ^ Looking good (no idea how it looked before, I've done this blind).

Let me know if I have to change something, and if yes what I have to change in the output of bl-kb to make things work smoothly at your end.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[ghost]

@thierrybo Thank you for your report; the issue is now fixed and will ship with the next package release (coming very soon).

@johnraff Unless you have any changes on your own to ship right now, I suggest you cut a new release of 'your' package.

[johanmalm]

@johnraff @2ion @hhhorb

For completeness, I've now pushed the commits to make this happen. openbox - wobbly jgmenu - straight

[johnraff]

@2ion I don't think there are any important changes waiting in bunsen-utilities but I'll do a quick check for any tweaks, update the changelog, push and tag today.

@johanmalm That looks very nice.

Thanks to everyone!

[johnraff]

Sorry, package upgrade has been a bit delayed by other things. Coming soon.