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

[19.09] nixos/libinput: apply options to all device types #73785

Merged
merged 1 commit into from Dec 3, 2019
Merged

[19.09] nixos/libinput: apply options to all device types #73785

merged 1 commit into from Dec 3, 2019

Conversation

worldofpeace
Copy link
Contributor

Motivation for this change

Without this change any special config used for libinput like accelSpeed won't apply to mice because it isn't a touchpad.

This could possibly change the user experience for someone, like if they came to rely on the bug.
An example situation would be:

  • have services.xserver.libinput.accelSpeed in config
  • not notice it doesn't apply to your device that isn't a touchpad (mouse)
  • update on 19.09 and suddenly the acceleration of your mouse is different.

It's generally not good to make a change like that, but it would be nice to be able to actually configure your mouse with libinput...

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

@NixOS/backports (and myself apparently)

@sylv-io @worldofpeace
nixos/libinput: apply options to all device types
ea83c79 
Instead of assign the libinput options to touchpad devices only, it
should be appied by any device using libinput.
Due to the fact that `40-libinput.conf` already defines libinput as
driver for any detected input device, we can use `MatchDriver` to appy
options.

(cherry picked from commit d616928)
@worldofpeace worldofpeace added the 8.has: port to stable A PR already has a backport to the stable release. label Nov 19, 2019
@worldofpeace worldofpeace mentioned this pull request Nov 19, 2019
Merged
10 tasks
@lheckemann
Copy link
Member

This stops libinput from being set as the driver for all touchpads, so I think we'd need to split that part into a new InputClass section.

I don't think this should go to 19.09 though, since as you said it changes existing behaviour and configuring mice using libinput is possible on 19.09 using services.xserver.inputClassSections.

@worldofpeace
Copy link
Contributor Author

This stops libinput from being set as the driver for all touchpads, so I think we'd need to split that part into a new InputClass section.

Regression ❔ (I'd definitely not want to backport that)

@lheckemann
Copy link
Member

Yes, and that should be fixed on master as well (has it already been merged to master?).

@worldofpeace
Copy link
Contributor Author

Yes, and that should be fixed on master as well (has it already been merged to master?).

If you look at the commit is says it has been cherry-picked.

Though what is confusing to me is that I tested the commit functioning, and llibinput list-devices still lists my touchpad etc.

@worldofpeace
Copy link
Contributor Author

We've established there was no issues at #72774 (comment).

@worldofpeace worldofpeace merged commit 72a2ced into NixOS:release-19.09 Dec 3, 2019
@worldofpeace worldofpeace deleted the libinput-options-all-19.09 branch December 3, 2019 02:09
@thiagokokada
Copy link
Contributor

Just to leave it here since this change surprised me. I was actually reliant of the fact that those options are only applied to a touchpad, since I want to enable natural scrolling in my touchpad, however not in my mouse. It was really difficult to understand what was happening.

While it maybe just me, maybe there is more people that had this same issue.

@thiagokokada thiagokokada mentioned this pull request Dec 5, 2019
Closed
@lheckemann
Copy link
Member

This is exactly why I didn't want it backported…

@thiagokokada
Copy link
Contributor

Well, actually my setup is broken. I can't set natural scrolling to work only in my touchpad and not in mouse. It is either all devices (using services.xserver.libinput.naturalScrolling = true) or nothing. I tried both services.xserver.extraConfig and services.xserver.inputClassSection and they both don't work.

Could we revert this PR @lheckemann @worldofpeace? Or at least, there is some solution for this problem? It is really awful to have natural scrolling for both mouse and touchpad, however touchpad without natural scrolling is very strange.

@thiagokokada
Copy link
Contributor

BTW, I have a hypothesis why this is either all or nothing. It is because NaturalScrolling option for libinput is always set, so even if you don't set services.xserver.libinput.naturalScrolling it will be set to false, applying this setting to all devices.

@jabranham
Copy link
Contributor

services.xserver.libinput.additionalOptions no longer seems to work for me. Its docstring says Additional options for libinput touchpad driver. I had it set to ''Option "Ignore" "on" '' to disable the touchpad on my laptop but that no longer does anything.

worldofpeace referenced this pull request Dec 20, 2019
@worldofpeace
Revert "nixos/libinput: apply options to all device types"
faffc22 
This reverts commit ea83c79.
@bakhtiyarneyman
Copy link

For what it's worth, I liked the new behavior so much that after the rollback I had to bring it back via inputClassSections. :)

@thiagokokada
Copy link
Contributor

@niobium0 The problem is that the change was implemented in a minor update. I think this new behavior will be available nonetheless in the next NixOS release.

@thiagokokada thiagokokada mentioned this pull request Dec 26, 2020
Closed
10 tasks
@thiagokokada thiagokokada mentioned this pull request Jan 10, 2021
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@worldofpeace @lheckemann @thiagokokada @jabranham @bakhtiyarneyman @sylv-io