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

Add Sections and MO(layer)/TG(layer) Example #6308

Merged
merged 3 commits into from
Jul 25, 2019

Conversation

thomas-m-d
Copy link
Contributor

Description

Changes:

  1. Added sub-section headings to the portion before the examples.
  2. Edited text around sub-headings to better fit with sub-headings.
  3. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

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

  • The FIXME comment at the top of docs/feature_tap_dance.md is addressed, and therefore deleted.

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).

My change is a documentation change. I have tested the code from my Example 6 using a MiniVan keyboard (see qmk_firmware/keyboards/thevankeyboards/minivan), and it works as it should. I believe my example code follows the code style of this project, but this is my first commit so I might have missed something.

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.
Copy link
Member

@noroadsleft noroadsleft 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.

@drashna
Copy link
Member

drashna commented Jul 11, 2019

Just a heads up:
#6259

There is some overlap/conflicts there, and I just wanted to make you aware. It's not merged yet, but it's a better way to handle the custom tapping term for tap dances.

@thomas-m-d
Copy link
Contributor Author

thomas-m-d commented Jul 11, 2019

@drashna Thanks for the pointer. I'm very new to GitHub, so please let me know if there are things I can do on my end to help out, and if so any pointers/search terms you could spare would be a huge help.

I'm guessing I should be the one to change the example so it's not using the deprecated function, and that should be an easy change on my end so I'll start working on that.

@thomas-m-d
Copy link
Contributor Author

@drashna I updated docs/feature_tap_dance.md to include your edits and to no longer use ACTION_TAP_DANCE_FN_ADVANCED_TIME in my example. I'll keep an eye out for when your work gets merged, and update my example then.

@noroadsleft Let me know if there's anything else I need to do.

Best!


The first option is enough for a lot of cases, that just want dual roles. For example, `ACTION_TAP_DANCE_DOUBLE(KC_SPC, KC_ENT)` will result in `Space` being sent on single-tap, `Enter` otherwise.
* ~~`ACTION_TAP_DANCE_FN_ADVANCED_TIME(on_each_tap_fn, on_dance_finished_fn, on_dance_reset_fn, tap_specific_tapping_term)`~~: This functions identically to the `ACTION_TAP_DANCE_FN_ADVANCED` function, but uses a custom tapping term for it, instead of the predefined `TAPPING_TERM`.
* This is deprecated in favor of the Per Key Tapping Term functionality, as outlined [here](custom_quantum_functions.md#Custom_Tapping_Term). You'd want to check for the specific `TD()` macro that you want to use (such as `TD(TD_ESC_CAPS)`) instead of using this specific.
Copy link
Member

Choose a reason for hiding this comment

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

Actually, for now, if you could remove the changes from my PR? This should be fine as it was, for the most part. And until my PR lands, this won't be accurate.

@thomas-m-d
Copy link
Contributor Author

I finally got to revert those changes. Sorry for the delay.

@drashna
Copy link
Member

drashna commented Jul 25, 2019

No worries. We know that not everyone has a lot of time to do stuff. Or it can be easy to get side tracked. :)

@drashna drashna merged commit a747953 into qmk:master Jul 25, 2019
raymond-w-ko pushed a commit to raymond-w-ko/qmk_firmware that referenced this pull request Aug 4, 2019
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
doughsay pushed a commit to doughsay/qmk_firmware that referenced this pull request Aug 31, 2019
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
swanmatch pushed a commit to swanmatch/qmk_firmware that referenced this pull request Sep 3, 2019
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
ridingqwerty pushed a commit to ridingqwerty/qmk_firmware that referenced this pull request Jan 10, 2020
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
swamp09 pushed a commit to swamp09/qmk_firmware that referenced this pull request Mar 11, 2020
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Add Sections and MO(layer)/TG(layer) Example

Major changes:
1. Added sub-section headings to the portion before the examples.
2. Added a new Example 6, that allows MO(layer) and TG(layer) functionality to be embedded within tap dance functions.

Minor Changes:
1. Edited some text to better fit with new sub-headings.

* Update feature_tap_dance.md

* Update feature_tap_dance.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants