BOTH_SHIFTS_TURNS_ON_CAPS_WORD and sm_td.c

[gagregrog]

Hey there!

I saw your post on Reddit last week and was very intrigued. I use a 3x5 layout and rely on HRM, but like others have struggled with accidental firings. My solution was to move my mod keys to the bottom row. While workable, it wasn't an ideal solution as it obviously changed how I typed and pulled me away from the home row.

So, when I saw your post I was excited and wanted to give it a try.

I have a few different keyboards that I use, so I share my code between them using a QMK userspace so that I get the same core experience on each keyboard. So, I began integrating sm_td into my userspace.

Before I get into the few surmountable issues I ran into, I want to say that I'm excited about this library and plan on test driving it for a while. Already it is a definite improvement over the built in mod taps, and I think with some more tweaking I'll be able to smooth out some of the kinks that are still slowing me down. So, thank you for sharing this with the community!

Issue 1: sm_td.h -> sm_td.c

After following your readme, I quickly came across a compile failure. I was getting errors for multiple definitions of all of the function and variables that were defined within sm_td.h. My understanding is that generally it is better to define these within a .c file, and not the header file, so that the header file can be included where needed without running into this problem.

So, I forked your repo and split things up between sm_td.h and sm_td.c. This route requires you to add an additional line to your rules.mk in order for the compiler to find the .c file, but I believe that generally this is more robust.

Issue 2: BOTH_SHIFTS_TURNS_ON_CAPS_WORD

I noticed that you added logic to ensure that letters are shifted when caps word is on, but I did not see anything in place that allowed you to utilize the BOTH_SHIFTS_TURNS_ON_CAPS_WORD feature. I've added some code that allows you to benefit from that feature to the SMTD_MT macro. I didn't implement this in the other macros as I'm currently only using SMTD_MT, and the other two were structured differently.

Other things

I also added a convenience macro SM_MT (and the other variants) that piggy backs on your macros, but accepts only the tap_key and automatically prefixes SM_ to it as the macro key. I found that for HRM I generally only needed to use a basic KC_ key in conjunction with a mod key, so having this shortcut made the configuration feel a little easier.

---

Anyways, thanks for the cool library, and feel free to take any of these changes if they speak to you. Otherwise, I'm happy to keep using as a fork.

You can view the diff here

Cheers

[stasmarkin]

Thank you very much for your contribution!
So, there are plenty things to discuss.

Issue 1: sm_td.h -> sm_td.c

I've chosen this way to compose a code because it seems to me it's much easier to install a single file for a user. I had some plans to split it into .c + .h files, but I'm not sure if it's really worth it.
Could you please elaborate in which cases you have multiple definition compilation errors? In my personal configurations, I have multiple includes of sm_td.h and have never encountered this problem. I believe `#pragma once' must handle this problem.

Issue 2: BOTH_SHIFTS_TURNS_ON_CAPS_WORD

I see the problem. I've never thought about it because I turn on caps-word feature with another macro. So yes, it will be really handy to support basic QMK flag in sm_td. Honestly, I don't really like the solution you did because SMTD_MT macro is more generic and I don't feel like it's the right place to check this. But I will definitely support BOTH_SHIFTS_TURNS_ON_CAPS_WORD in next releases.

I also added a convenience macro SM_MT (and the other variants) that piggy backs on your macros, but accepts only the tap_key and automatically prefixes SM_

I recently came up with a solution that doesn't require creating custom keycodes at all. So sm_td will work on top of the basic KC_* keycodes. I think this will simplify a lot of things, including macros and the installation process.

Anyway, I'm very happy to see someone fork and improve this library, so thank you very much for your contribution, I will try my best to incorporate your suggestions!

[gagregrog]

Issue 1: sm_td.h -> sm_td.c

So, as I understand it the #pragma once does as you subscribe if you include it in multiple header files _within the same compilation unit-, but not if it is included in multiple .c files. I'm definitely not an expert here (I barely scrape by in c/cpp), but there are a number of Stack overflow posts that support this claim:

• https://stackoverflow.com/questions/14422872/pragma-once-doesnt-prevent-multiple-variable-definitions
• https://stackoverflow.com/questions/249701/why-arent-my-compile-guards-preventing-multiple-definition-inclusions
• https://stackoverflow.com/questions/223771/repeated-multiple-definition-errors-from-including-same-header-in-multiple-cpps/223798

In my case, I include sm_td.h in my <my_user>.h userspace entrypoint. That userspace file is then included in all of my other userspace libraries to share keymaps, layouts, and other common features. Perhaps this is not the proper way to do this, and if you know of a better way please let me know, but due to this structure I encounter the multiple definition errors from the linker.

Issue 2: BOTH_SHIFTS_TURNS_ON_CAPS_WORD

Happy to have this code triggered elsewhere if you've found a cleaner solution :-)

I recently came up with a solution that doesn't require creating custom keycodes at all. So sm_td will work on top of the basic KC_* keycodes.

This sounds great! Will this solution also support advanced keycodes? For example, LCAG(KC_RIGHT)? With how I'm using your library (and how I've used built in mod taps in the past), I've had to rely on a custom keycode as a flag to then trigger the advanced keycode.

---

Thanks for your responses!

[stasmarkin]

Will this solution also support advanced keycodes? For example, LCAG(KC_RIGHT)?

I hope so :) I'm trying to do my best here