Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Add keyval_to/is_upper/lower functions to Key struct #358

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

Amjad50
Copy link
Contributor

@Amjad50 Amjad50 commented Sep 30, 2020

added:

  • keyval_is_upper
  • keyval_is_lower
  • keyval_to_upper
  • keyval_to_lower

As I could not use them in my project in the recent versions, here, As u32 cannot be converted back into Key

what do you think? @GuillaumeGomez

Copy link
Member

@sdroege sdroege left a comment

Choose a reason for hiding this comment

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

Looks good to me, but maybe we should also add From<u32> for Key and From<Key> for u32 impls? That's basically there already anyway via the DerefMut / Deref impls

Is there any reason why this could cause problems?

@Amjad50
Copy link
Contributor Author

Amjad50 commented Sep 30, 2020

oops, actually it appears I can convert without this PR:

let mut val = event.get_keyval();
*val = keyval_to_upper(*val);
match val {
...

I don't know why I struggled a lot for converting it to uppercase, that's why I suggested this PR.
Do you think this would be good anyway? I think its better for code clarity.

we should also add From<u32> for Key and From<Key> for u32 impls?

Yes, though I think From<Key> for u32 is not needed as it is identical to Deref

@sdroege
Copy link
Member

sdroege commented Sep 30, 2020

Do you think this would be good anyway? I think its better for code clarity.

I agree, yes

src/keys.rs Outdated Show resolved Hide resolved
@Amjad50 Amjad50 force-pushed the key_upper_lower_functions branch from b10c4bd to 15b126c Compare September 30, 2020 08:38
@Amjad50
Copy link
Contributor Author

Amjad50 commented Sep 30, 2020

I guess keyval_name, keyval_to_unicode are needed in lib.rs, they are being exported to outside

@Amjad50
Copy link
Contributor Author

Amjad50 commented Sep 30, 2020

since now u32 can be converted to Key, are keyval_name(u32), keyval_to_unicode(u32) necessary?

@GuillaumeGomez
Copy link
Member

I don't think we should keep them.

@Amjad50 Amjad50 force-pushed the key_upper_lower_functions branch 2 times, most recently from 58aa1cf to f298ca5 Compare September 30, 2020 10:03
@Amjad50
Copy link
Contributor Author

Amjad50 commented Sep 30, 2020

problems with check_init_asserts?

@GuillaumeGomez
Copy link
Member

You might need to add some skip_assert_initialized where it's missing.

@Amjad50 Amjad50 force-pushed the key_upper_lower_functions branch from f298ca5 to 2038357 Compare September 30, 2020 13:36
@sdroege sdroege merged commit fbb38fd into gtk-rs:master Sep 30, 2020
@Amjad50 Amjad50 deleted the key_upper_lower_functions branch September 30, 2020 16:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants