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

Allow Alt + mouse wheel up/down to scroll apps #130

Closed
wants to merge 4 commits into from
Closed

Allow Alt + mouse wheel up/down to scroll apps #130

wants to merge 4 commits into from

Conversation

mckellyln
Copy link

Hi,
Thanks to all for Terminator!
Here is a PR for the small change discussed before in:
#46
to allow Alt + mouse wheel up/down to scroll.
I wanted to make it so I could check for Alt + wheel in application and scroll 2x/4x more but that is still to be determined how. This at least gets us to scroll normally.
thx,
-m

@mattrose
Copy link
Member

Thanks for the contribution. I haven't forgotten about you, it's just that my current "setup" (as the kids would say) is sitting on my couch with my laptop, and I haven't dug out my mouse recently to test your changes. I'll test and merge as soon as I can.

@mckellyln
Copy link
Author

No problem :-)

@@ -1018,6 +1018,9 @@ def on_mousewheel(self, widget, event):
elif event.direction == Gdk.ScrollDirection.DOWN or SMOOTH_SCROLL_DOWN:
self.scroll_by_page(1)
return (True)
if event.state & Gdk.ModifierType.MOD1_MASK == Gdk.ModifierType.MOD1_MASK:

Choose a reason for hiding this comment

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

Gdk.ModifierType.MOD1_MASK == Gdk.ModifierType.MOD1_MASK would always evaluate to true so it can be removed from this conditional

Copy link
Author

@mckellyln mckellyln Jun 22, 2020

Choose a reason for hiding this comment

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

ok, sure. I was copying the coding style of the other event.state conditionals near by.
I'm not sure leaving the '== Gdk.ModifierType.MOD1_MASK' off or changing it to != 0 helps readability ?
Or is even still correct as without the == MOD1_MASK it could mean other modifiiers are also set/pressed ?

Copy link

@GrzegorzDerdak GrzegorzDerdak Jun 22, 2020

Choose a reason for hiding this comment

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

Changing it to != 0 would be better. Also, I just notice it, please use and instead of & - it's more pythonic. So the full conditional could be just if event.state and Gdk.ModifierType.MOD1_MASK != 0 unless Gdk.ModifierType.MOD1_MASK could have any value than 0 then if event.state and Gdk.ModifierType.MOD1_MASK is just fine.

Copy link
Author

Choose a reason for hiding this comment

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

Is 'and' bitwise ? Not sure how it can be both bitwise and logical ? I think its pretty common to use '&' for bitwise ?
Would parens help ?
Again I'm just following the same style as several other lines above it.

Choose a reason for hiding this comment

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

Ok, my mistake. Maybe just leave as it is since other lines are in the same style. Sorry for the confusion.

@@ -1018,6 +1018,9 @@ def on_mousewheel(self, widget, event):
elif event.direction == Gdk.ScrollDirection.DOWN or SMOOTH_SCROLL_DOWN:
self.scroll_by_page(1)
return (True)
if event.state & Gdk.ModifierType.MOD1_MASK == Gdk.ModifierType.MOD1_MASK:

Choose a reason for hiding this comment

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

Ok, my mistake. Maybe just leave as it is since other lines are in the same style. Sorry for the confusion.

@mckellyln
Copy link
Author

Closing as its probably too far behind master now and its such a small change.
thx,
-m

@mckellyln mckellyln closed this Jul 22, 2020
@mckellyln mckellyln deleted the alt_wheel_scroll branch July 22, 2020 19:41
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.

3 participants