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(core): include nul in context offset calculations in kmx processor #13314

Open
wants to merge 1 commit into
base: chore/core/13303-unit-tests-for-nul-index-context
Choose a base branch
from

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Feb 21, 2025

Two separate bugs addressed, with index() references and with context() references -- both have the same root cause, of not taking nul at the start of the context into account (as nul is not included in the m_miniContext member, being a non-character). We already fixed this issue for if() quite a long time ago, and some of the same patterns can be seen with m_miniContextIfLen for example.

Fixes: #13304
Fixes: #13316

@keymanapp-test-bot skip

@keymanapp-test-bot keymanapp-test-bot bot added this to the B18S2 milestone Feb 21, 2025
@github-actions github-actions bot added core/ Keyman Core fix labels Feb 21, 2025
Two separate bugs addressed, with `index()` references and with
`context()` references -- both have the same root cause, of not taking
`nul` at the start of the context into account (as `nul` is not included
in the `m_miniContext` member, being a non-character). We already fixed
this issue for `if()` quite a long time ago, and some of the same
patterns can be with `m_miniContextIfLen` for example.

Fixes: #13304
Fixes: #13316
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@rc-swag rc-swag left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/ Keyman Core fix
Projects
Status: No status
3 participants