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

Fix space cadet auto shift interaction #24440

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AndrewMoscoe
Copy link

@AndrewMoscoe AndrewMoscoe commented Sep 26, 2024

Fix for this bug: #20978

Enable space cadet shift - left shift (, right shift )
Enable auto shift

Press shift, press a letter, and release shift before releasing the letter.

When using left shift, the ( will be before the letter. When using right shift, the ) will be after the letter.

If you release shift after releasing the letter, the () will not print, and the keyboard works as you would want.

Description

Fix space cadet-auto shift interaction bug

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • [x ] My code follows the code style of this project: C, Python
  • [ x] I have read the PR Checklist document and have made the appropriate changes.
  • 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).

@github-actions github-actions bot added the core label Sep 26, 2024
@drashna
Copy link
Member

drashna commented Sep 26, 2024

Could you (please) add to the unit tests, as well? They are there to help ensure that behavior is consistent and as expected, especially when things change.

@AndrewMoscoe
Copy link
Author

AndrewMoscoe commented Sep 28, 2024

Could you (please) add to the unit tests, as well? They are there to help ensure that behavior is consistent and as expected, especially when things change.

I added it, but the CI job says "This workflow is awaiting approval from a maintainer"

@IsaacElenbaas
Copy link
Contributor

Just wanted to link #24341 - you might run into it since you're also using physical shift keys.

@drashna
Copy link
Member

drashna commented Oct 8, 2024

Could you (please) add to the unit tests, as well? They are there to help ensure that behavior is consistent and as expected, especially when things change.

I added it, but the CI job says "This workflow is awaiting approval from a maintainer"

"first time contributor", github security thing. It can take a bit before approval is given.
s
That said, you can run the tests locally. Either with qmk test-c -t auto_shift or make test:autoshift

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Lint wants:

tests/auto_shift/test_auto_shift.cpp Show resolved Hide resolved
tests/auto_shift/test_auto_shift.cpp Outdated Show resolved Hide resolved
@drashna drashna self-requested a review October 15, 2024 19:24
tests/auto_shift/test_auto_shift.cpp Outdated Show resolved Hide resolved
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.

[Bug] Space Cadet Shift and Auto Shift interaction produces unwanted () characters
3 participants