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

[Keymap] Update brauner preonic layout #20439

Merged
merged 1 commit into from
Apr 16, 2023
Merged

Conversation

brauner
Copy link
Contributor

@brauner brauner commented Apr 13, 2023

Adapt to changes that make IGNORE_MOD_TAP_INTERRUPT the default.

Comment on lines 284 to 287
case MOD_TAP_LSFT_ESC:
/* Immediately select the hold action when another key is pressed. */
return true;
case MOD_TAP_LSFT_ENT:
/* Immediately select the hold action when another key is pressed. */
return true;
Copy link
Contributor

@elpekenin elpekenin Apr 13, 2023

Choose a reason for hiding this comment

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

Not a big difference, but writing it like this reduces repetition a bit, and would help adding other keycodes here easier and faster in the future

Suggested change
case MOD_TAP_LSFT_ESC:
/* Immediately select the hold action when another key is pressed. */
return true;
case MOD_TAP_LSFT_ENT:
/* Immediately select the hold action when another key is pressed. */
return true;
case MOD_TAP_LSFT_ESC:
case MOD_TAP_LSFT_ENT:
/* Immediately select the hold action when another key is pressed. */
return true;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with that change. Note, that if you do allow that in the code you probably want to use fallthrough semantics going forward, i.e.,

  #if HAVE_COMPILER_ATTR_FALLTHROUGH || __GNUC__ >= 7
  #define fallthrough __attribute__((__fallthrough__))
  #else
  #define fallthrough /* fall through */
  #endif

  bool get_hold_on_other_key_press(uint16_t keycode, keyrecord_t *record) {
      switch (keycode) {
          case MOD_TAP_LSFT_ESC:
              fallthrough;
          case MOD_TAP_LSFT_ENT:
              /* Immediately select the hold action when another key is pressed. */
              return true;
          default:
              /* Do not select the hold action when another key is pressed. */
              return false;
      }
  }

Then all places can be converted to use fallthrough and then you can ultimately enable -Wimplicit-fallthrough=5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@drashna drashna requested a review from a team April 15, 2023 09:49
@waffle87 waffle87 merged commit 1e3d2f2 into qmk:develop Apr 16, 2023
coquizen pushed a commit to coquizen/qmk_firmware that referenced this pull request Jun 22, 2023
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.

4 participants