Improve consistency in UNICODEMAP code and docs, update docs/understanding_qmk #4774

vomindoraan · 2019-01-04T12:42:58Z

Rename some constants and functions pertaining to UNICODEMAP so they're consistent with the rest of the system. Also update the documentation accordingly. See below for details.

These changes should be non-breaking. I've taken care to update all usages of the renamed symbols.

Description

Remove unused UNICODE(n) macro (alias for UC(n), but no one was using it).
Rename QK_UNICODE_MAP constant to QK_UNICODEMAP (only used internally).
Rename unicode_map_input_error function to unicodemap_input_error (only used internally).
Rename process_unicode_map function to process_unicodemap (only used internally).

Reasoning: UNICODEMAP is the name of the keymap definition method (hence UNICODEMAP_ENABLE, process_unicodemap etc.), whereas unicode_map refers to the mapping table itself, which is used by the method.
Refactor process_unicodemap code a bit to make it more readable.
Update links in docs/understanding_qmk and rearrange the list of process record functions so it matches the actual order in which they're called now (the order was slightly altered in Overhaul Unicode Common functionality #4325).
Various Unicode doc improvements.

Tested by compiling several keymaps that use UNICODEMAP.

P.S. When I say it's only used internally, I mean it's only used by QMK's internals and not directly in any userspaces/keymaps. That's why I think it's okay to rename these symbols. No “user facing” symbols were altered in the making of this PR 😛

Types of changes

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. (https://docs.qmk.fm/#/contributing)
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).

This is done for consistency: UNICODEMAP is the method (hence UNICODEMAP_ENABLE, process_unicodemap), whereas unicode_map is the mapping table itself.

This avoids the issue of the compiler sometimes complaining about the array index being out of range

vomindoraan · 2019-01-05T02:23:00Z

@drashna I'd appreciate a review, fellow Unicodee 😄

drashna · 2019-01-05T16:36:13Z

quantum/quantum_keycodes.h

 #ifdef UNICODEMAP_ENABLE
-    #define X(n) (QK_UNICODE_MAP | (n))
+  // Allows Unicode input up to 0x10FFFF, requires unicode_map
+  #define X(i) (QK_UNICODEMAP | (i))


I forgot to add this myself, but I'd like to see an "else" for both of these (UC and X) that define them as empty functions, so they don't error out when compiling when the respective feature is disabled.

They could be defined as KC_NONE in that case. But what's the use case? It seems to me like you'd want compilation to fail as fast as possible if you put UC or X in your keymap but forget to enable the corresponding feature.

I'm not sure that failing is the best option here, to be honest.

Two reasons really come up for this:

Community Layouts

EEPROM storage of keymaps

Also, testing comes to mind, but that's an edge case.

The alternative is that if I want to do this locally, I would have to "ifdef" the keycodes ANYWAYS.

But for now, I'll just merge this.

Could you elaborate on the community layouts bit? I think I know what you mean, but it'd be best if you could clarify just in case.

As far as keymap storage goes, UC and X themselves aren't part of the keycodes enum (they're just macros), so leaving them undefined doesn't affect other keycodes. What does get left out is QK_UNICODE and QK_UNICODEMAP, but if I'm not mistaken, those should be left out for the sake of feature flagging.

drashna · 2019-01-07T20:56:45Z

Thanks!

* 'master' of https://github.com/qmk/qmk_firmware: (63 commits) Keymap: ave-63's iris layout (qmk#4812) Update bakingpy 4x12 keymap and add test mode for debugging/development (qmk#4810) Fix Mac mute keycode (qmk#4809) Add KBD75 keymap (qmk#4808) Fix pinout of split hand and LED, remove flip half option Tidy up Mod-Tap shortcuts (qmk#4806) Add missing parentheses to some important macros (qmk#4775) Keyboard: Downbubble refactor and Configurator fix (qmk#4798) Alternate keymap for Alpha keyboard, enjoy! (qmk#4797) Keymap: Added Model F-inspired layout for YMD[K]96 (qmk#4777) Improve consistency in UNICODEMAP code and docs, update docs/understanding_qmk (qmk#4774) Update to arm_atsam wait and timer routines Add Downbubble to Handwired repository (qmk#4794) Final HS60v2 changes. (qmk#4790) Keyboard: Fractal layout macro and readme cleanup (qmk#4789) Keymap: added my espectro keymap (qmk#4791) Keyboard: Numbrero: Configurator fix and code tidy (qmk#4787) Keyboard: Tradestation code tidy and readme refactor (qmk#4784) Keyboard: update readme with ps2avr flashing instructions (qmk#4776) add Pinky keyboard (qmk#4748) ...

…nding_qmk (qmk#4774) * Remove unused UNICODE(n) macro, update docs * Add note about max length of unicode_map to docs * QK_UNICODE_MAP → QK_UNICODEMAP * Refactor process_unicode_map * process_unicode_map → process_unicodemap This is done for consistency: UNICODEMAP is the method (hence UNICODEMAP_ENABLE, process_unicodemap), whereas unicode_map is the mapping table itself. * Update references and ordering in docs/understanding_qmk * Add additional note to docs/understanding_qmk * &unicode_map[index] → unicode_map + index This avoids the issue of the compiler sometimes complaining about the array index being out of range * Update docs/getting_started_make_guide * Update method sections in docs/feature_unicode

vomindoraan added 7 commits January 4, 2019 12:29

Remove unused UNICODE(n) macro, update docs

dfc541f

Add note about max length of unicode_map to docs

5a7109b

QK_UNICODE_MAP → QK_UNICODEMAP

dc6afb7

Refactor process_unicode_map

a7db100

process_unicode_map → process_unicodemap

e1203a2

This is done for consistency: UNICODEMAP is the method (hence UNICODEMAP_ENABLE, process_unicodemap), whereas unicode_map is the mapping table itself.

Update references and ordering in docs/understanding_qmk

1d200b6

Add additional note to docs/understanding_qmk

b6ebc56

vomindoraan force-pushed the unicodemap_consistency branch from 781e2b5 to 8877605 Compare January 4, 2019 16:06

&unicode_map[index] → unicode_map + index

ca04a30

This avoids the issue of the compiler sometimes complaining about the array index being out of range

vomindoraan force-pushed the unicodemap_consistency branch from 8877605 to ca04a30 Compare January 5, 2019 00:40

vomindoraan added 2 commits January 5, 2019 03:14

Update docs/getting_started_make_guide

c9e0354

Update method sections in docs/feature_unicode

244ac8f

drashna reviewed Jan 5, 2019

View reviewed changes

mechmerlin added optimization documentation labels Jan 5, 2019

drashna requested review from skullydazed, jackhumbert, ezuk, fredizzimo and mechmerlin January 5, 2019 18:03

drashna approved these changes Jan 5, 2019

View reviewed changes

drashna merged commit cd9262d into qmk:master Jan 7, 2019

vomindoraan deleted the unicodemap_consistency branch January 8, 2019 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve consistency in UNICODEMAP code and docs, update docs/understanding_qmk #4774

Improve consistency in UNICODEMAP code and docs, update docs/understanding_qmk #4774

vomindoraan commented Jan 4, 2019 •

edited

Loading

vomindoraan commented Jan 5, 2019

drashna Jan 5, 2019

vomindoraan Jan 6, 2019 •

edited

Loading

drashna Jan 7, 2019

drashna Jan 7, 2019

vomindoraan Jan 7, 2019

drashna commented Jan 7, 2019

Improve consistency in UNICODEMAP code and docs, update docs/understanding_qmk #4774

Improve consistency in UNICODEMAP code and docs, update docs/understanding_qmk #4774

Conversation

vomindoraan commented Jan 4, 2019 • edited Loading

Description

Types of changes

Checklist:

vomindoraan commented Jan 5, 2019

drashna Jan 5, 2019

Choose a reason for hiding this comment

vomindoraan Jan 6, 2019 • edited Loading

Choose a reason for hiding this comment

drashna Jan 7, 2019

Choose a reason for hiding this comment

drashna Jan 7, 2019

Choose a reason for hiding this comment

vomindoraan Jan 7, 2019

Choose a reason for hiding this comment

drashna commented Jan 7, 2019

vomindoraan commented Jan 4, 2019 •

edited

Loading

vomindoraan Jan 6, 2019 •

edited

Loading