[Bug] CAPS_WORD: '-' with hold OSL gives _, but with tap OSL gives -

[mmccoyd]

Describe the Bug

With CAPS_WORD off:

• hold or tap OSL, tap - gives -

With CAPS_WORD on:

• hold OSL, tap - gives _
• tap OSL, tap - gives -

To reproduce:

Keymap with -_ on a OSL.

• Hold OSL, tap -
• Tap OSL, tap -

Activate CAPS_WORD

• Type some letters to verify CAPS_WOD
• Hold OSL, tap -
• Tap OSL, tap -
• Type some letters

Results:

OFF: HH--HH
ON:  HH_-HH

Tested on

Master and dev with Hillside 52 default_dot_c keymap, a vanilla keymap,
with MO(_SYM) changed to OSL(_SYM)

https://github.com/qmk/qmk_firmware/blob/master/keyboards/handwired/hillside/52/keymaps/default_dot_c/keymap.c

System Information

Keyboard: Hillside 52
Revision (if applicable):
Operating system: OS X
qmk doctor output:

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.0.0
Ψ QMK home: /Users/xxxxxx/qmk_firmware
Ψ Detected macOS 11.6.8 (Intel).
Ψ Git branch: develop
Ψ Repo version: 0.17.9
⚠ Git has unstashed/uncommitted changes.
⚠ The official repository does not seem to be configured as git remote "upstream".
Ψ CLI installed in virtualenv.
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 8.3.1
Ψ Found avr-gcc version 8.4.0
Ψ Found avrdude version 6.3
Ψ Found dfu-util version 0.11
Ψ Found dfu-programmer version 0.7.2
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2022-06-11 09:09:45 +0000 --  (f836d24b0)
Ψ - lib/chibios-contrib: 2022-07-27 10:46:15 +0200 --  (d03aa9cc)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-06-12 22:00:28 +1000 --  (35cc3d92f)
Ψ - lib/pico-sdk: 2022-05-17 19:19:01 +0200 --  (07edde8)
Ψ - lib/printf: 2022-06-29 23:59:58 +0300 --  (c2e3b4e)
Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 --  (819dbc1)
Ψ QMK is ready to go, but minor problems were found

Git diff

(py39)  qmk_firmware_hillside_fork % git diff
diff --git a/keyboards/handwired/hillside/52/keymaps/default_dot_c/keymap.c b/keyboards/handwired/hillside/52/keymaps/default_dot_c/keymap.c
index f0c5b755d5..88b379baba 100644
--- a/keyboards/handwired/hillside/52/keymaps/default_dot_c/keymap.c
+++ b/keyboards/handwired/hillside/52/keymaps/default_dot_c/keymap.c
@@ -13,7 +13,7 @@ enum layers {
 #define xxxxxxx KC_NO
 
 #define LY_NAV MO(_NAV)
-#define LY_SYM MO(_SYM)
+#define LY_SYM OSL(_SYM)
 #define LY_ADJ MO(_ADJUST)
 #define ALT_GR OSM(MOD_RALT)
 #define OSM_SFT OSM(MOD_LSFT)

Any keyboard related software installed?

• AutoHotKey (Windows)
• Karabiner (macOS)
• Other:

Additional Context

[mmccoyd]

Apparently CAPS_WORD changes - into _ on base layer. Which is fine, but not in the docs.
But the inconsistency between OSL tap vs hold remains.
Unless it is an optimization to get - more easily.

[mmccoyd]

Or, as precondition said more exactly:

CAPS_WORD down
CAPS_WORD up
OSL down
OSL up
KC_MINS down
KC_MINS up

produces -

CAPS_WORD down
CAPS_WORD up
OSL down
KC_MINS down
KC_MINS up
OSL up

produces _ underscore

[precondition]

diff --git a/tests/caps_word/test_caps_word.cpp b/tests/caps_word/test_caps_word.cpp
index 3f59ed3744..ef689c6458 100644
--- a/tests/caps_word/test_caps_word.cpp
+++ b/tests/caps_word/test_caps_word.cpp
@@ -233,6 +233,58 @@ TEST_F(CapsWord, SpaceTurnsOffCapsWord) {
     testing::Mock::VerifyAndClearExpectations(&driver);
 }
 
+// Tests that holding a OSL keeps caps word active and shifts keys on the layer that need to be shifted.
+TEST_F(CapsWord, IgnoresOSLHold) {
+    TestDriver driver;
+    KeymapKey key_a(0, 0, 0, KC_A);
+    KeymapKey key_osl(0, 1, 0, OSL(1));
+    KeymapKey key_b(1, 0, 0, KC_B);
+    set_keymap({key_a, key_osl, key_b});
+
+    // Allow any number of reports with no keys or only modifiers.
+    // clang-format off
+    EXPECT_CALL(driver, send_keyboard_mock(AnyOf(
+                KeyboardReport(),
+                KeyboardReport(KC_LSFT))))
+        .Times(AnyNumber());
+
+    EXPECT_REPORT(driver, (KC_LSFT, KC_B));
+    caps_word_on();
+
+    key_osl.press();
+    run_one_scan_loop();
+    tap_key(key_b);
+    key_osl.release();
+    run_one_scan_loop();
+
+    testing::Mock::VerifyAndClearExpectations(&driver);
+}
+
+// Tests that tapping a OSL keeps caps word active and shifts keys on the layer that need to be shifted.
+TEST_F(CapsWord, IgnoresOSLTap) {
+    TestDriver driver;
+    KeymapKey key_a(0, 0, 0, KC_A);
+    KeymapKey key_osl(0, 1, 0, OSL(1));
+    KeymapKey key_b(1, 0, 0, KC_B);
+    set_keymap({key_a, key_osl, key_b});
+
+    // Allow any number of reports with no keys or only modifiers.
+    // clang-format off
+    EXPECT_CALL(driver, send_keyboard_mock(AnyOf(
+                KeyboardReport(),
+                KeyboardReport(KC_LSFT))))
+        .Times(AnyNumber());
+
+    EXPECT_REPORT(driver, (KC_LSFT, KC_B));
+    caps_word_on();
+
+    tap_key(key_osl);
+    tap_key(key_b);
+    run_one_scan_loop();
+
+    testing::Mock::VerifyAndClearExpectations(&driver);
+}
+

For convenience to a future developer who wants to fix the issue, I quickly wrote a unit-test to check for this bug. IgnoresOSLHold passes but IgnoresOSLTap fails, as described in this issue.

[drashna]

Is this still happening on the 0.19.x version. There are some changes that may... help with this.

[precondition]

At least, the unit tests I wrote now pass.