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

Ergodox EZ refactor #3171

Merged
merged 4 commits into from
Jun 13, 2018
Merged

Ergodox EZ refactor #3171

merged 4 commits into from
Jun 13, 2018

Conversation

noroadsleft
Copy link
Member

Here's me attempting to figure out why the Ergodox layouts in the Configurator don't render properly.

I've had a theory about the API not liking comments in the matrix macros for a while, so this is me testing that.

@@ -250,8 +250,8 @@ void x_reset (qk_tap_dance_state_t *state, void *user_data);
// Since our quirky block definitions are basically a list of comma separated
// arguments, we need a wrapper in order for these definitions to be
// expanded before being used as arguments to the LAYOUT_xxx macro.
#if (!defined(LAYOUT) && defined(KEYMAP))
#define LAYOUT KEYMAP
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't compile, at least not for me, without changing it.

With these changes, there is no KEYMAP macro. There's only LAYOUT_ergodox, LAYOUT_ergodox_80, and the pretty versions of those to choose from. (I need to make another commit to delete the alias lines in ergodox_ez.h.)

If you have a better way of achieving the desired result, I'm all ears.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, because the keymap.c should have LAYOUT_ergodox_wrapper rather than LAYOUT_wrapper

@drashna
Copy link
Member

drashna commented Jun 13, 2018

Also, you may want to change layouts/community/ergodox/berfarah/keymap.c

@noroadsleft
Copy link
Member Author

layouts/community/ergodox/berfarah/keymap.c is edited in the first commit (the one without the Travis log).

@drashna
Copy link
Member

drashna commented Jun 13, 2018

Ah, sorry, missed it.

But the userspace one shouldn't be changed. It's "correct" for what it's doing.

@@ -20,7 +20,7 @@

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {

[DVORAK] = LAYOUT_wrapper (
[DVORAK] = LAYOUT_wrapper(
Copy link
Member

Choose a reason for hiding this comment

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

Should be LAYOUT_ergodox_wrapper, if anything.

@drashna
Copy link
Member

drashna commented Jun 13, 2018

Awesome, thanks

@drashna drashna merged commit 698ce73 into qmk:master Jun 13, 2018
@noroadsleft noroadsleft deleted the c10r-ergodox branch June 13, 2018 05:13
noj added a commit to noj/qmk_firmware that referenced this pull request Jun 16, 2018
…jan_atreus

* 'master' of https://github.com/qmk/qmk_firmware: (578 commits)
  Contra refactor (qmk#3191)
  Fixed no-op ifndef for ergodox ez (qmk#3189)
  Canoe Refactor (qmk#3190)
  planck/light/info.json fix (qmk#3186)
  Tada68 refactor and readme update (qmk#3178)
  Add URL of PCB files to README.md (qmk#3182)
  Rename from KEYMAP to LAYOUT (qmk#3181)
  Fix bfake matrix bug (qmk#3180)
  Clueboard 2x1800 Refactor (qmk#3179)
  QMK Configurator Support for Melody96 (qmk#3177)
  Lightsaver info.json update (qmk#3176)
  KBD66 info.json update (qmk#3175)
  Mbsurfer deltasplit75 keymap (qmk#3174)
  Configurator layout repair for Ergodone, Ergodox Infinity and ErgoTravel (qmk#3173)
  Ergodox EZ refactor (qmk#3171)
  Hadron: Readme, Refactor, and Configurator support (qmk#3170)
  fix issue with rgbinit unused variable (qmk#3165)
  Fix melody96 default keymap graphic. (qmk#3169)
  Fix jj40 capslock and minor keymap updates (qmk#3168)
  GON NerD: Refactor, Configurator support and Readme cleanup (qmk#3167)
  ...
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
* Refactor matrices and keymaps

* Fixes for @EricGebhart keymap

* Deleted macro aliases

* ericgebhart/keymap.c to LAYOUT_ergodox_wrapper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants