niri: fix some keybind tab issues

- Fix args for screenshot
- move-column stuff is focus=true by default
- Parsing fixes
part of #914

core/internal/keybinds/providers/niri.go

@@ -371,6 +371,18 @@ func (n *NiriProvider) buildActionNode(action string) *document.Node {
 
 	node.SetName(parts[0])
 	for _, arg := range parts[1:] {
+		if strings.Contains(arg, "=") {
+			kv := strings.SplitN(arg, "=", 2)
+			switch kv[1] {
+			case "true":
+				node.AddProperty(kv[0], true, "")
+			case "false":
+				node.AddProperty(kv[0], false, "")
+			default:
+				node.AddProperty(kv[0], kv[1], "")
+			}
+			continue
+		}
 		node.AddArgument(arg, "")
 	}
 	return node
@@ -379,7 +391,7 @@ func (n *NiriProvider) buildActionNode(action string) *document.Node {
 func (n *NiriProvider) parseActionParts(action string) []string {
 	var parts []string
 	var current strings.Builder
-	var inQuote, escaped bool
+	var inQuote, escaped, wasQuoted bool
 
 	for _, r := range action {
 		switch {
@@ -389,17 +401,19 @@ func (n *NiriProvider) parseActionParts(action string) []string {
 		case r == '\\':
 			escaped = true
 		case r == '"':
+			wasQuoted = true
 			inQuote = !inQuote
 		case r == ' ' && !inQuote:
-			if current.Len() > 0 {
+			if current.Len() > 0 || wasQuoted {
 				parts = append(parts, current.String())
 				current.Reset()
+				wasQuoted = false
 			}
 		default:
 			current.WriteRune(r)
 		}
 	}
-	if current.Len() > 0 {
+	if current.Len() > 0 || wasQuoted {
 		parts = append(parts, current.String())
 	}
 	return parts
@@ -508,6 +522,10 @@ func (n *NiriProvider) writeBindNode(sb *strings.Builder, bind *overrideBind, in
 			sb.WriteString(" ")
 			n.writeArg(sb, arg.ValueString(), forceQuote)
 		}
+		if child.Properties.Exist() {
+			sb.WriteString(" ")
+			sb.WriteString(strings.TrimLeft(child.Properties.String(), " "))
+		}
 	}
 	sb.WriteString("; }\n")
 }

core/internal/keybinds/providers/niri_parser.go

@@ -265,6 +265,11 @@ func (p *NiriParser) parseKeybindNode(node *document.Node, _ string) *NiriKeyBin
 		for _, arg := range actionNode.Arguments {
 			args = append(args, arg.ValueString())
 		}
+		if actionNode.Properties != nil {
+			if val, ok := actionNode.Properties.Get("focus"); ok {
+				args = append(args, "focus="+val.String())
+			}
+		}
 	}
 
 	var description string

core/internal/keybinds/providers/niri_parser_test.go

@@ -602,8 +602,24 @@ func TestNiriParseActionWithProperties(t *testing.T) {
 	for _, kb := range result.Section.Keybinds {
 		switch kb.Action {
 		case "move-column-to-workspace":
-			if len(kb.Args) != 1 {
+			if len(kb.Args) != 2 {
-				t.Errorf("move-column-to-workspace should have 1 arg, got %d", len(kb.Args))
+				t.Errorf("move-column-to-workspace should have 2 args (index + focus), got %d", len(kb.Args))
+			}
+			hasIndex := false
+			hasFocus := false
+			for _, arg := range kb.Args {
+				if arg == "1" || arg == "2" {
+					hasIndex = true
+				}
+				if arg == "focus=false" {
+					hasFocus = true
+				}
+			}
+			if !hasIndex {
+				t.Errorf("move-column-to-workspace missing index arg")
+			}
+			if !hasFocus {
+				t.Errorf("move-column-to-workspace missing focus=false arg")
 			}
 		case "next-window":
 			if kb.Key != "Tab" {

quickshell/Common/KeybindActions.js

@@ -197,14 +197,26 @@ const ACTION_ARGS = {
             { name: "focus", type: "bool", label: "Follow focus", default: false }
         ]
     },
+    "move-column-to-workspace-down": {
+        args: [{ name: "focus", type: "bool", label: "Follow focus", default: false }]
+    },
+    "move-column-to-workspace-up": {
+        args: [{ name: "focus", type: "bool", label: "Follow focus", default: false }]
+    },
     "screenshot": {
-        args: [{ name: "opts", type: "screenshot", label: "Options" }]
+        args: [{ name: "show-pointer", type: "bool", label: "Show pointer" }]
     },
     "screenshot-screen": {
-        args: [{ name: "opts", type: "screenshot", label: "Options" }]
+        args: [
+            { name: "show-pointer", type: "bool", label: "Show pointer" },
+            { name: "write-to-disk", type: "bool", label: "Save to disk" }
+        ]
     },
     "screenshot-window": {
-        args: [{ name: "opts", type: "screenshot", label: "Options" }]
+        args: [
+            { name: "show-pointer", type: "bool", label: "Show pointer" },
+            { name: "write-to-disk", type: "bool", label: "Save to disk" }
+        ]
     }
 };
 
@@ -288,11 +300,12 @@ function getActionLabel(action) {
     if (!action)
         return "";
 
-    const dmsAct = findDmsAction(action);
+    var dmsAct = findDmsAction(action);
     if (dmsAct)
         return dmsAct.label;
 
-    const compAct = findCompositorAction(action);
+    var base = action.split(" ")[0];
+    var compAct = findCompositorAction(base);
     if (compAct)
         return compAct.label;
 
@@ -337,7 +350,8 @@ function isValidAction(action) {
 function isKnownCompositorAction(action) {
     if (!action)
         return false;
-    return findCompositorAction(action) !== null;
+    var base = action.split(" ")[0];
+    return findCompositorAction(base) !== null;
 }
 
 function buildSpawnAction(command, args) {
@@ -404,10 +418,10 @@ function parseCompositorActionArgs(action) {
     if (!ACTION_ARGS[base])
         return { base: action, args: {} };
 
-    var argConfig = ACTION_ARGS[base];
     var argParts = parts.slice(1);
 
-    if (base === "move-column-to-workspace") {
+    switch (base) {
+    case "move-column-to-workspace":
         for (var i = 0; i < argParts.length; i++) {
             if (argParts[i] === "focus=true" || argParts[i] === "focus=false") {
                 args.focus = argParts[i] === "focus=true";
@@ -415,14 +429,24 @@ function parseCompositorActionArgs(action) {
                 args.index = argParts[i];
             }
         }
-    } else if (base.startsWith("screenshot")) {
+        break;
-        args.opts = {};
+    case "move-column-to-workspace-down":
-        for (var j = 0; j < argParts.length; j += 2) {
+    case "move-column-to-workspace-up":
-            if (j + 1 < argParts.length)
+        for (var k = 0; k < argParts.length; k++) {
-                args.opts[argParts[j]] = argParts[j + 1];
+            if (argParts[k] === "focus=true" || argParts[k] === "focus=false")
+                args.focus = argParts[k] === "focus=true";
+        }
+        break;
+    default:
+        if (base.startsWith("screenshot")) {
+            for (var j = 0; j < argParts.length; j++) {
+                var kv = argParts[j].split("=");
+                if (kv.length === 2)
+                    args[kv[0]] = kv[1] === "true";
+            }
+        } else if (argParts.length > 0) {
+            args.value = argParts.join(" ");
         }
-    } else if (argParts.length > 0) {
-        args.value = argParts.join(" ");
     }
 
     return { base: base, args: args };
@@ -437,24 +461,29 @@ function buildCompositorAction(base, args) {
     if (!args || Object.keys(args).length === 0)
         return base;
 
-    if (base === "move-column-to-workspace") {
+    switch (base) {
+    case "move-column-to-workspace":
         if (args.index)
             parts.push(args.index);
-        if (args.focus === true)
+        if (args.focus === false)
-            parts.push("focus=true");
-        else if (args.focus === false)
             parts.push("focus=false");
-    } else if (base.startsWith("screenshot") && args.opts) {
+        break;
-        for (var key in args.opts) {
+    case "move-column-to-workspace-down":
-            if (args.opts[key] !== undefined && args.opts[key] !== "") {
+    case "move-column-to-workspace-up":
-                parts.push(key);
+        if (args.focus === false)
-                parts.push(args.opts[key]);
+            parts.push("focus=false");
-            }
+        break;
+    default:
+        if (base.startsWith("screenshot")) {
+            if (args["show-pointer"] === true)
+                parts.push("show-pointer=true");
+            if (args["write-to-disk"] === true)
+                parts.push("write-to-disk=true");
+        } else if (args.value) {
+            parts.push(args.value);
+        } else if (args.index) {
+            parts.push(args.index);
         }
-    } else if (args.value) {
-        parts.push(args.value);
-    } else if (args.index) {
-        parts.push(args.index);
     }
 
     return parts.join(" ");

quickshell/Modules/Settings/KeybindsTab.qml

@@ -597,7 +597,7 @@ Item {
                             onSaveBind: (originalKey, newData) => {
                                 KeybindsService.saveBind(originalKey, newData);
                                 keybindsTab._editingKey = newData.key;
-                                keybindsTab.expandedKey = modelData.action;
+                                keybindsTab.expandedKey = newData.action;
                             }
                             onRemoveBind: key => {
                                 const remainingKey = bindItem.keys.find(k => k.key !== key)?.key ?? "";

quickshell/Widgets/KeybindItem.qml

@@ -650,9 +650,10 @@ Item {
                             }
 
                             onWheel: wheel => {
-                                if (!root.recording)
+                                if (!root.recording) {
+                                    wheel.accepted = false;
                                     return;
-
+                                }
                                 wheel.accepted = true;
 
                                 const mods = [];
@@ -959,12 +960,12 @@ Item {
                         Layout.preferredWidth: 120
                         compactMode: true
                         currentValue: {
-                            const action = root.editAction;
+                            const base = root.editAction.split(" ")[0];
                             const cats = KeybindsService.getCompositorCategories();
                             for (const cat of cats) {
                                 const actions = KeybindsService.getCompositorActions(cat);
                                 for (const act of actions) {
-                                    if (act.id === action)
+                                    if (act.id === base)
                                         return cat;
                                 }
                             }
@@ -1024,12 +1025,13 @@ Item {
                 }
 
                 RowLayout {
+                    id: optionsRow
                     Layout.fillWidth: true
                     spacing: Theme.spacingM
                     visible: root._actionType === "compositor" && !root.useCustomCompositor && Actions.getActionArgConfig(root.editAction)
 
-                    property var argConfig: Actions.getActionArgConfig(root.editAction)
+                    readonly property var argConfig: Actions.getActionArgConfig(root.editAction)
-                    property var parsedArgs: Actions.parseCompositorActionArgs(root.editAction)
+                    readonly property var parsedArgs: Actions.parseCompositorActionArgs(root.editAction)
 
                     StyledText {
                         text: I18n.tr("Options")
@@ -1048,56 +1050,75 @@ Item {
                             Layout.fillWidth: true
                             Layout.preferredHeight: 40
                             visible: {
-                                const cfg = parent.parent.argConfig;
+                                const cfg = optionsRow.argConfig;
-                                if (!cfg || !cfg.config || !cfg.config.args)
+                                if (!cfg?.config?.args)
                                     return false;
                                 const firstArg = cfg.config.args[0];
                                 return firstArg && (firstArg.type === "text" || firstArg.type === "number");
                             }
-                            placeholderText: {
+                            placeholderText: optionsRow.argConfig?.config?.args?.[0]?.placeholder || ""
-                                const cfg = parent.parent.argConfig;
+
-                                if (!cfg || !cfg.config || !cfg.config.args)
+                            Connections {
-                                    return "";
+                                target: optionsRow
-                                return cfg.config.args[0]?.placeholder || "";
+                                function onParsedArgsChanged() {
+                                    const newText = optionsRow.parsedArgs?.args?.value || optionsRow.parsedArgs?.args?.index || "";
+                                    if (argValueField.text !== newText)
+                                        argValueField.text = newText;
+                                }
                             }
-                            text: parent.parent.parsedArgs?.args?.value || parent.parent.parsedArgs?.args?.index || ""
+
-                            onTextChanged: {
+                            Component.onCompleted: {
-                                const cfg = parent.parent.argConfig;
+                                text = optionsRow.parsedArgs?.args?.value || optionsRow.parsedArgs?.args?.index || "";
+                            }
+
+                            onEditingFinished: {
+                                const cfg = optionsRow.argConfig;
                                 if (!cfg)
                                     return;
-                                const base = parent.parent.parsedArgs?.base || root.editAction.split(" ")[0];
+                                const parsed = optionsRow.parsedArgs;
-                                const args = cfg.config.args[0]?.type === "number" ? {
+                                const args = {};
-                                    index: text
+                                if (cfg.config.args[0]?.type === "number")
-                                } : {
+                                    args.index = text;
-                                    value: text
+                                else
-                                };
+                                    args.value = text;
+                                if (parsed?.args?.focus === false)
+                                    args.focus = false;
                                 root.updateEdit({
-                                    action: Actions.buildCompositorAction(base, args)
+                                    action: Actions.buildCompositorAction(parsed?.base || cfg.base, args)
                                 });
                             }
                         }
 
                         RowLayout {
                             visible: {
-                                const cfg = parent.parent.argConfig;
+                                const cfg = optionsRow.argConfig;
-                                return cfg && cfg.base === "move-column-to-workspace";
+                                if (!cfg)
+                                    return false;
+                                switch (cfg.base) {
+                                case "move-column-to-workspace":
+                                case "move-column-to-workspace-down":
+                                case "move-column-to-workspace-up":
+                                    return true;
+                                }
+                                return false;
                             }
                             spacing: Theme.spacingXS
 
                             DankToggle {
                                 id: focusToggle
-                                checked: parent.parent.parent.parsedArgs?.args?.focus === true
+                                checked: optionsRow.parsedArgs?.args?.focus !== false
-                                onCheckedChanged: {
+                                onToggled: newChecked => {
-                                    const cfg = parent.parent.parent.argConfig;
+                                    const cfg = optionsRow.argConfig;
                                     if (!cfg)
                                         return;
-                                    const parsed = parent.parent.parent.parsedArgs;
+                                    const parsed = optionsRow.parsedArgs;
-                                    const args = {
+                                    const args = {};
-                                        index: parsed?.args?.index || "",
+                                    if (cfg.base === "move-column-to-workspace")
-                                        focus: checked
+                                        args.index = parsed?.args?.index || "";
-                                    };
+                                    if (!newChecked)
+                                        args.focus = false;
                                     root.updateEdit({
-                                        action: Actions.buildCompositorAction("move-column-to-workspace", args)
+                                        action: Actions.buildCompositorAction(cfg.base, args)
                                     });
                                 }
                             }
@@ -1110,53 +1131,22 @@ Item {
                         }
 
                         RowLayout {
-                            visible: {
+                            visible: optionsRow.argConfig?.base?.startsWith("screenshot") ?? false
-                                const cfg = parent.parent.argConfig;
-                                return cfg && cfg.base && cfg.base.startsWith("screenshot");
-                            }
                             spacing: Theme.spacingM
 
-                            RowLayout {
-                                spacing: Theme.spacingXS
-
-                                DankToggle {
-                                    id: writeToDiskToggle
-                                    checked: parent.parent.parent.parent.parsedArgs?.args?.opts?.["write-to-disk"] === "true"
-                                    onCheckedChanged: {
-                                        const parsed = parent.parent.parent.parent.parsedArgs;
-                                        const base = parsed?.base || "screenshot";
-                                        const opts = parsed?.args?.opts || {};
-                                        opts["write-to-disk"] = checked ? "true" : "";
-                                        root.updateEdit({
-                                            action: Actions.buildCompositorAction(base, {
-                                                opts: opts
-                                            })
-                                        });
-                                    }
-                                }
-
-                                StyledText {
-                                    text: I18n.tr("Save")
-                                    font.pixelSize: Theme.fontSizeSmall
-                                    color: Theme.surfaceVariantText
-                                }
-                            }
-
                             RowLayout {
                                 spacing: Theme.spacingXS
 
                                 DankToggle {
                                     id: showPointerToggle
-                                    checked: parent.parent.parent.parent.parsedArgs?.args?.opts?.["show-pointer"] === "true"
+                                    checked: optionsRow.parsedArgs?.args?.["show-pointer"] === true
-                                    onCheckedChanged: {
+                                    onToggled: newChecked => {
-                                        const parsed = parent.parent.parent.parent.parsedArgs;
+                                        const parsed = optionsRow.parsedArgs;
                                         const base = parsed?.base || "screenshot";
-                                        const opts = parsed?.args?.opts || {};
+                                        const args = Object.assign({}, parsed?.args || {});
-                                        opts["show-pointer"] = checked ? "true" : "";
+                                        args["show-pointer"] = newChecked;
                                         root.updateEdit({
-                                            action: Actions.buildCompositorAction(base, {
+                                            action: Actions.buildCompositorAction(base, args)
-                                                opts: opts
-                                            })
                                         });
                                     }
                                 }
@@ -1167,6 +1157,31 @@ Item {
                                     color: Theme.surfaceVariantText
                                 }
                             }
+
+                            RowLayout {
+                                visible: optionsRow.argConfig?.base !== "screenshot"
+                                spacing: Theme.spacingXS
+
+                                DankToggle {
+                                    id: writeToDiskToggle
+                                    checked: optionsRow.parsedArgs?.args?.["write-to-disk"] === true
+                                    onToggled: newChecked => {
+                                        const parsed = optionsRow.parsedArgs;
+                                        const base = parsed?.base || "screenshot-screen";
+                                        const args = Object.assign({}, parsed?.args || {});
+                                        args["write-to-disk"] = newChecked;
+                                        root.updateEdit({
+                                            action: Actions.buildCompositorAction(base, args)
+                                        });
+                                    }
+                                }
+
+                                StyledText {
+                                    text: I18n.tr("Save")
+                                    font.pixelSize: Theme.fontSizeSmall
+                                    color: Theme.surfaceVariantText
+                                }
+                            }
                         }
                     }
                 }