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

[Enhancement] More info on apply_autocorrect #21056

Merged
merged 5 commits into from
Jul 7, 2023

Conversation

elpekenin
Copy link
Contributor

Description

The information received on the hook was not very useful because it only showed part of global state, namely how to fix it. However it provided no information on what the typo being solved was.

Thus we now also provide both the wrong and corrected versions of the strings.

Also updated its only usage over the repo.

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

Checklist

  • My code follows the code style of this project: C, Python
  • 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).

@elpekenin elpekenin changed the title [Enhancement] More info on autocorrect's user hook [Enhancement] More info on apply_autocorrect May 26, 2023
@drashna drashna requested a review from a team May 26, 2023 21:15
@drashna
Copy link
Member

drashna commented May 29, 2023

found a bug. It looks like it will grab text from before the word to correct:

Autocorrected layeqpayed to layeqpaid
Autocorrected payed to paid
Autocorrected laypaiedpayed to laypaiedpaid

Mostly was trying to trigger "payed", but if you don't have a space before the word, it will include everything up til the last space/terminator character.

@elpekenin
Copy link
Contributor Author

elpekenin commented May 29, 2023

Not a bug, but a limitation on how the algorithm works

It has no concept of "words" , we just have a stream of chars in which we find for patterns to be replaced (since we can use spaces to hopefully correctly delimit words, my code does so)

I dont think there's a way of fixing this issue without major changes to the correction code

PS: Anyway it doesnt feel like a very common scenario to run into

@drashna
Copy link
Member

drashna commented May 29, 2023

Yeah. And this issue isn't a stopping issue. However, I think that because this change makes this more obvious, a note about this behavior should be added as it will likely come up more often if/when people can render the "word".

@tzarc tzarc merged commit 55295ed into qmk:develop Jul 7, 2023
@elpekenin elpekenin deleted the feature/autocorrect_strings branch July 7, 2023 14:15
jesperhellberg pushed a commit to jesperhellberg/qmk_firmware that referenced this pull request Sep 9, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
thismarvin pushed a commit to thismarvin/qmk_firmware that referenced this pull request Sep 27, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
akeep pushed a commit to akeep/qmk_firmware that referenced this pull request Oct 2, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
csolje pushed a commit to csolje/qmk_firmware that referenced this pull request Oct 21, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
autoferrit pushed a commit to SpaceRockMedia/bastardkb-qmk that referenced this pull request Dec 8, 2023
Co-authored-by: Drashna Jaelre <drashna@live.com>
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