nixos/libinput: Make every option optional #107676
Conversation
|
|
|
Tagging @worldofpeace since its is the author of #73785. |
|
Also, I don't think this is the best solution. Something better would be to have separate sections for each type of input class in {
xserver.libinput.touchpad = {
naturallScrolling = true;
};
xserver.libinput.mouse = {
accelProfile = "flat";
};
}If someone things that the above is a good idea, I can give it a try too. |
|
Marking @zackelan @pmyjavec @karthikiyengar, since they commented in issue #75007 and maybe can help review/validate this. |
thiagokokada
changed the title
ofborg
bot
added
10.rebuild-darwin: 0
10.rebuild-linux: 1-10
10.rebuild-linux: 1
labels
|
So I applied the changes (using {
services = {
xserver = {
enable = true;
libinput.enable = true;
inputClassSections = [
''
Identifier "mouse"
MatchIsPointer "on"
Option "AccelProfile" "flat"
''
''
Identifier "touchpad"
MatchIsTouchpad "on"
Option "NaturalScrolling" "on"
Option "Tapping" "on"
Option "TappingButtonMap" "lmr"
''
];
};
};
}This seems to work, I have natural scrolling enabled in my touchpad but not in my mouse. Just to make sure that all devices are running with |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
The libinput module nowadays set some options as optional (i.e.: accepts null) and other options as required (i.e.: if not set it will be set with a default). However, this module, since PR #73785, applies to all input devices. And this is an issue since many of the options here only make sense to touchpad (for example, NaturalScrolling). Worse than that, once any option is in this configuration it cannot be overriden by any of the existing mechanisms to override, like: - `services.xserver.extraConfig` - `services.xserver.inputClassSections` The only one that works is `services.xserver.libinput.additionalOptions`, but this one does not support applying options to many targets. You can set `additionalOptions` with `MatchIsTouchpad "on"`, for example, but now you cannot apply options in other target unless you use one of the options above. So what this PR does is convert every option to optional, and also setting to `null` any option that is already default in libinput itself. This way, most options will be available to override using `services.xserver.extraConfig` or `services.xserver.inputClassSections`. Closes #75007.
|
/marvin opt-in |
|
Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here. |
|
@thiagokokada I have a question! ✨ do you know how this could affect Wayland compositors using libinput, e.g. sway? Can you explain how can I maybe test it on my system, what settings could I try to tweak to see if there's a useful difference in behavior there, or maybe bugs? |
@kisik21 Shouldn't affect Wayland, since Wayland doesn't use Xorg.conf AFAIK. SwayWM for example uses its own completely custom configuration file for input devices: https://github.com/swaywm/sway/wiki#input-configuration.
Well, the easiest way I found to test modules is something like this: { ... }:
let
libinputModuleRefactor = fetchTarball {
url = "https://github.com/thiagokokada/nixpkgs/archive/a27a2ff9a3605a9ddc740a3c3fa3d51392fde040.tar.gz";
sha256 = "..."; # get the correct SHA for commit
};
in {
imports = [
"${libinputModuleRefactor}/nixos/modules/services/x11/hardware/libinput.nix"
];
disabledModules = [
"services/x11/hardware/libinput.nix"
];
xserver.libinput.enable = true;
# Set some custom X config here so you can test the changes too.
# This comment may help: https://github.com/NixOS/nixpkgs/pull/107676#issuecomment-752844528
}Maybe there is someway easier to test this (for example, I can try to write some tests if this would make the review easier), but this is what I found the easiest way to test it. |
|
For what behavior you're testing to see if this fix it, try this: {
services = {
xserver = {
enable = true;
libinput.enable = true;
inputClassSections = [
''
Identifier "mouse"
MatchIsPointer "on"
Option "AccelProfile" "flat"
''
''
Identifier "touchpad"
MatchIsTouchpad "on"
Option "NaturalScrolling" "on"
''
];
};
};
}Now this is slightly trick to test since you need a notebook with a touchpad and a external mouse. But what will happen in Now with my branch, it should work as expected: touchpad will have natural scrolling and mouse will have accel profile flat. More details can be seen in #75007. |
|
I will close this PR in favor of #108909, because it seems a better solution. |
Motivation for this change
The libinput module nowadays set some options as optional (i.e.: accepts null) and other options as required (i.e.: if not set it will be set with a default).
However, this module, since PR #73785, applies to all input devices. And this is an issue since many of the options here only make sense to touchpad (for example, NaturalScrolling). Worse than that, once any option is in this configuration it cannot be overriden by any of the existing mechanisms to override, like:
services.xserver.extraConfigservices.xserver.inputClassSectionsThe only one that works is
services.xserver.libinput.additionalOptions, but this one does not support applying options to many targets. You can setadditionalOptionswithMatchIsTouchpad "on", for example, but now you cannot apply options in other target unless you use one of the options above.So what this PR does is convert every option to optional, and also setting to
nullany option that is already default in libinput itself. This way, most options will be available to override usingservices.xserver.extraConfigorservices.xserver.inputClassSections.Closes #75007.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)