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.

Sign up for GitHub

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

Jump to bottom

Standardize how unicode is processed (fixes #8768) #8770

Merged
merged 11 commits into from
Jun 18, 2020
Merged

Standardize how unicode is processed (fixes #8768) #8770

merged 11 commits into from
Jun 18, 2020

Conversation

Str8AWay
Copy link
Contributor

Description

  • Standardize how unicode is processed by utilizing how unicodemap converts it for OS X and move that to process_unicode_common.
  • Add the ability to have multiple characters sent with UCIS

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@drashna drashna requested a review from a team April 12, 2020 22:25
@drashna
Copy link
Member

drashna commented Apr 12, 2020

@vomindoraan ?

vomindoraan
vomindoraan suggested changes May 7, 2020
View reviewed changes
Copy link
Contributor

@vomindoraan vomindoraan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The idea behind this PR is solid. There's still room for improvement, though.

Most of the bellow suggestions are interrelated, so please evaluate and commit them all at once.

Beyond these changes, I would also recommend renaming qk_ucis_state_t.codes to qk_ucis_state_t.keycodes, for clarity and to further differentiate it from qk_ucis_symbol_t.code_points.
Also, the unicode_input_cancel() function can probably be removed now, since it's no longer used.
However, these two changes are best left for a different PR.

quantum/process_keycode/process_unicode_common.c Outdated Show resolved Hide resolved
quantum/process_keycode/process_unicodemap.c Outdated Show resolved Hide resolved
quantum/process_keycode/process_unicodemap.c Outdated Show resolved Hide resolved
quantum/process_keycode/process_unicode_common.h Outdated Show resolved Hide resolved
docs/feature_unicode.md Outdated Show resolved Hide resolved
quantum/process_keycode/process_ucis.c Outdated Show resolved Hide resolved
quantum/process_keycode/process_ucis.c Outdated Show resolved Hide resolved
quantum/process_keycode/process_ucis.h Outdated Show resolved Hide resolved
quantum/process_keycode/process_ucis.h Outdated Show resolved Hide resolved
docs/feature_unicode.md Show resolved Hide resolved
@vomindoraan vomindoraan mentioned this pull request May 7, 2020
Merged
@vomindoraan
Copy link
Contributor

vomindoraan commented May 7, 2020 •
edited
Loading

I've implemented the above suggestions in Str8AWay#1, along with a few others that I couldn't add through GitHub's suggestion system. If you merge that PR, this PR will automatically be updated with my suggested changes.

@Str8AWay
Copy link
Contributor Author

Str8AWay commented May 7, 2020

Thanks for taking a look and the suggestions. I merged your PR with your suggestions

@vomindoraan
Copy link
Contributor

Great, feel free to mark all of my above comments as resolved.

vomindoraan
vomindoraan approved these changes May 7, 2020
View reviewed changes
Copy link
Contributor

@vomindoraan vomindoraan left a comment

Choose a reason for hiding this comment

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

LGTM. Successfully compiles with @algernon's Ergodox keymap, but I haven't tested it.

@vomindoraan vomindoraan mentioned this pull request May 10, 2020
Merged
@Str8AWay
Merge pull request #2 from vomindoraan/StandardUnicodeImplementation_…
b211a55 
…patch

StandardUnicodeImplementation_patch
@Str8AWay
Copy link
Contributor Author

Anything I can do to help get this merged?

@Str8AWay Str8AWay requested a review from fauxpark May 29, 2020 13:25
@drashna drashna requested a review from a team June 3, 2020 05:17
@Erovia Erovia merged commit f7eb030 into qmk:master Jun 18, 2020
@vomindoraan vomindoraan mentioned this pull request Jun 18, 2020
Merged
13 tasks
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
@Str8AWay @vomindoraan @jakobaa
Standardize how unicode is processed (fixes qmk#8768) (qmk#8770)
928d0df 
Co-authored-by: Konstantin Đorđević <[email protected]>
@sigprof sigprof mentioned this pull request Jul 29, 2020
Closed
3 tasks
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Aug 9, 2020
@Str8AWay @vomindoraan @drashna
Standardize how unicode is processed (fixes qmk#8768) (qmk#8770)
bbc3012 
Co-authored-by: Konstantin Đorđević <[email protected]>
@spidey3 spidey3 mentioned this pull request Dec 15, 2020
Merged
14 tasks
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@Str8AWay @vomindoraan
Standardize how unicode is processed (fixes qmk#8768) (qmk#8770)
95a5c01 
Co-authored-by: Konstantin Đorđević <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vomindoraan vomindoraan vomindoraan approved these changes

@fauxpark fauxpark fauxpark approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
awaiting review core enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] UCIS doesn't support all unicode values for Mac OS X
7 participants
@Str8AWay @drashna @vomindoraan @fauxpark @Erovia @ridingqwerty @noroadsleft