linebender / glazier

Deprecated Rust Window Creation Library
Apache License 2.0
206 stars 32 forks source link

Stop wrapping keyboard_types::Modifiers. #133

Closed waywardmonkeys closed 1 year ago

waywardmonkeys commented 1 year ago

This was being wrapped to add a small number of extension methods which can be added via a trait instead.

The resulting code is smaller and makes it clear what is different from the upstream keyboard_types::Modifiers.

waywardmonkeys commented 1 year ago

I think there's a reasonable argument to be made that we can get rid of ModifiersExt as mod.shift() should really be mod.is_shift() and could just as easily be mod.contains(Modifiers::SHIFT) and we don't really lose much, if anything.

DJMcNab commented 1 year ago

I think we should still keep the methods - the long way is exceedingly noisy

is_shift to me suggests an exclusion of all other cases (as in is_none for example). But in this, you can be is_shift and is_ctrl, which doesn't seem right to me.

waywardmonkeys commented 1 year ago

Okay. I won't pursue that part further and I did find the issue upstream about this as well. But I'd still like to land this PR as it is.

DJMcNab commented 1 year ago

In https://github.com/pyfisch/keyboard-types/issues/7, it looks like a PR to add these methods upstream would be accepted, but doesn't exist yet.

Although the author appears to be no longer active, unfortunately. I suspect keyboard-types might need to move under servo?

waywardmonkeys commented 1 year ago

I've got an email drafted to one of the winit folks suggesting it move to rust-windowing