Skip to content

Fix key modifiers - #33

Open
crondog wants to merge 1 commit into
breach:masterfrom
crondog:fixkeyboard
Open

Fix key modifiers#33
crondog wants to merge 1 commit into
breach:masterfrom
crondog:fixkeyboard

Conversation

@crondog

@crondog crondog commented Jul 12, 2014

Copy link
Copy Markdown

I end up receiving the event modifier events starting from 1024. Masking fixes this. Is there a nicer way of doing this?

@m910q

m910q commented Jul 12, 2014

Copy link
Copy Markdown

Thanks, this fixes the non-working shortcuts in Ubuntu.

I guess it would be better to pre-calculate the value, instead of doing it in each if-statement.

@crondog

crondog commented Jul 13, 2014

Copy link
Copy Markdown
Author

So i found a nice way of doing it. I ended up creating Mod vars to make things easier to read. From what i have seen elsewhere doing it inside the if is common practice.

Could a mac person please test this as i changed the mac modifier code

@spolu

spolu commented Jul 13, 2014

Copy link
Copy Markdown
Member

I'll test it on both OSX and Linux on Monday!

@spolu

spolu commented Jul 14, 2014

Copy link
Copy Markdown
Member

Looks not bad, can you comply with the coding style (no camelcase).
Maybe we could define a function instead of a bunch of variable names, to test the the masks?

@spolu

spolu commented Jul 14, 2014

Copy link
Copy Markdown
Member

Works well on OSX! :)

ps2: Use Mod vars and clean up mac overrides
ps3: Fix var names
@crondog

crondog commented Jul 15, 2014

Copy link
Copy Markdown
Author

Ok I have fixed the var names to be lower case. Also I'm not really sure how we would use a function here instead the the variable names because wouldn't we still need to pass in and/or test that the event key modifier is what we want.

@spolu

spolu commented Jul 15, 2014

Copy link
Copy Markdown
Member

It could take the event as imput, an array of modifiers and a key?

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