feat: add XKB settings to input manager - #116
Conversation
Reviewer's GuideAdds version-2 XKB layout, model, variant, and options configuration to keyboard settings, with atomic pending updates and state-reporting events, while preserving compatibility through versioned protocol additions and updated documentation. Sequence diagram for XKB keyboard settings update and state reportingsequenceDiagram
participant Client
participant KeyboardSettings as KeyboardSettings_v2
participant Compositor
Compositor->>KeyboardSettings: xkb_rules(layout, model, variant, options)
Client->>KeyboardSettings: set_xkb_rules(layout, model, variant, options)
Client->>KeyboardSettings: apply()
KeyboardSettings->>Compositor: apply pending XKB rules snapshot
Compositor-->>KeyboardSettings: xkb_rules(layout, model, variant, options)
KeyboardSettings-->>Client: xkb_rules(layout, model, variant, options)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Adds XKB layout, model, variant, and options configuration to treeland_keyboard_settings_v1, including corresponding state events. The new requests and events are introduced in interface version 2 while preserving compatibility with existing clients. Log: add xkb settings to input manager PMS: BUG-377251 Influence: Compatible change
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zccrs, zzxyb The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dde/treeland-input-manager-unstable-v1.xml" line_range="790-797" />
<code_context>
+ request, does not by itself cause an xkb_rules event to be
+ emitted.
+ </description>
+ <arg name="layout" type="string" allow-null="true"
+ summary="comma-separated XKB layout names, or empty for the default"/>
+ <arg name="model" type="string" allow-null="true"
+ summary="XKB keyboard model, or empty for the default"/>
+ <arg name="variant" type="string" allow-null="true"
+ summary="comma-separated XKB variants, or empty for none"/>
+ <arg name="options" type="string" allow-null="true"
+ summary="comma-separated XKB options, or empty for none"/>
+ </request>
+
</code_context>
<issue_to_address>
**issue (bug_risk):** All four request arguments are declared nullable, but the protocol defines semantics only for empty strings and never specifies what a null layout, model, variant, or options value means. Clients that send the valid null form therefore cannot interoperate reliably, because compositors can interpret it differently from the documented empty-string defaults.
**Triggers:** When a client uses null for one or more XKB rules arguments.
**Suggested fix:** Either remove `allow-null="true"` and require the documented empty-string representation, or explicitly define null semantics for every argument and make the `xkb_rules` event representation consistent.
```suggestion
<arg name="layout" type="string"
summary="comma-separated XKB layout names, or empty for the default"/>
<arg name="model" type="string"
summary="XKB keyboard model, or empty for the default"/>
<arg name="variant" type="string"
summary="comma-separated XKB variants, or empty for none"/>
<arg name="options" type="string"
summary="comma-separated XKB options, or empty for none"/>
```
</issue_to_address>| <arg name="layout" type="string" allow-null="true" | ||
| summary="comma-separated XKB layout names, or empty for the default"/> | ||
| <arg name="model" type="string" allow-null="true" | ||
| summary="XKB keyboard model, or empty for the default"/> | ||
| <arg name="variant" type="string" allow-null="true" | ||
| summary="comma-separated XKB variants, or empty for none"/> | ||
| <arg name="options" type="string" allow-null="true" | ||
| summary="comma-separated XKB options, or empty for none"/> |
There was a problem hiding this comment.
issue (bug_risk): All four request arguments are declared nullable, but the protocol defines semantics only for empty strings and never specifies what a null layout, model, variant, or options value means. Clients that send the valid null form therefore cannot interoperate reliably, because compositors can interpret it differently from the documented empty-string defaults.
Triggers: When a client uses null for one or more XKB rules arguments.
Suggested fix: Either remove allow-null="true" and require the documented empty-string representation, or explicitly define null semantics for every argument and make the xkb_rules event representation consistent.
| <arg name="layout" type="string" allow-null="true" | |
| summary="comma-separated XKB layout names, or empty for the default"/> | |
| <arg name="model" type="string" allow-null="true" | |
| summary="XKB keyboard model, or empty for the default"/> | |
| <arg name="variant" type="string" allow-null="true" | |
| summary="comma-separated XKB variants, or empty for none"/> | |
| <arg name="options" type="string" allow-null="true" | |
| summary="comma-separated XKB options, or empty for none"/> | |
| <arg name="layout" type="string" | |
| summary="comma-separated XKB layout names, or empty for the default"/> | |
| <arg name="model" type="string" | |
| summary="XKB keyboard model, or empty for the default"/> | |
| <arg name="variant" type="string" | |
| summary="comma-separated XKB variants, or empty for none"/> | |
| <arg name="options" type="string" | |
| summary="comma-separated XKB options, or empty for none"/> |
Adds XKB layout, model, variant, and options configuration to treeland_keyboard_settings_v1, including corresponding state events.
The new requests and events are introduced in interface version 2 while preserving compatibility with existing clients.
Log: add xkb settings to input manager
PMS: BUG-377251
Influence: Compatible change
Summary by Sourcery
Extend keyboard settings with versioned XKB rules configuration and state reporting.
New Features:
Enhancements:
Documentation: