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.

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

Additional input options #91

Merged
merged 2 commits into from
Sep 25, 2021
Merged

Conversation

flaviozavan
Copy link
Contributor

Adds the possibility to configure disabling the input device while typing and
allows for choosing the click method. Both of which are useful for touchpads


Two points to consider:

  1. I wasn't sure if the way I set the default values is within expectations or not;
  2. I couldn't figure out how to run the tests.

Adds the possibility to configure disabling the input device while typing and
allows for choosing the click method. Both of which are useful for touchpads
Copy link
Owner

@inclement inclement left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much, looks great except some minor tweaks.

The tests needed manually starting as that seems to be the default for a first PR from someone, but I think they should now run automatically on future commits.

config/config.toml Outdated Show resolved Hide resolved
config/viv_config.h Outdated Show resolved Hide resolved
config/viv_config.h Outdated Show resolved Hide resolved
src/viv_toml_config.c Outdated Show resolved Hide resolved
@inclement
Copy link
Owner

I wasn't sure if the way I set the default values is within expectations or not;

I think ideally these should be pulled out to some centralised defaults list, but I hadn't thought about it before (and it didn't come up with the other libinput values) so there isn't an obvious one right now. Happy to merge as-is and change it later, I've made #92 to think about it.

That said, I think there's a case for making both options disabled by default - dwt in particular can be annoying for some (although essential for others, it's just subjective and hardware-dependent). Still happy to merge as-is, but I might push to change the defaults later.

Rename `disable-with-typing` to `disable-while-typing`, so it matches the
original DWT definitions. Reverts unusual defaults to 0/none/false. And fix
typo in click method alternatives
@flaviozavan
Copy link
Contributor Author

Addressed the comments. About the defaults. I don't really care about the ones I chose, they were simply my touchpad's default and I wanted to change them. I made it default to none/disabled now. The centralized defaults idea seems like a pretty good idea.

I figured out how to run the tests locally this time. So they should pass now.

@inclement inclement merged commit e861fe1 into inclement:main Sep 25, 2021
@inclement
Copy link
Owner

Great, thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants