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

bug(web): nul-prefixed contexts do not properly match corresponding rules #12987

Open
jahorton opened this issue Jan 22, 2025 · 1 comment · May be fixed by #12998
Open

bug(web): nul-prefixed contexts do not properly match corresponding rules #12987

jahorton opened this issue Jan 22, 2025 · 1 comment · May be fixed by #12998
Assignees
Milestone

Comments

@jahorton
Copy link
Contributor

jahorton commented Jan 22, 2025

While diagnosing #12985, which involves the sil_greek_polytonic keyboard, I noticed that certain rules weren't firing when expected. After digging down into things, it appears that Web does not currently handle rules of the following form correctly:

nul ______ + [ key ] > c anything

Rules that are of this form, however, are fine:

nul + [ key ] > c anything

Checking into our repo's history... this has likely been in place since 10.0 and is due to the following sections of code:

In both cases, should a context containing non-nul components have slots containing nul at their head... it just... chops it off, causing a misalignment for the method designed to match that context in a rule. I'm the original author of the block, to be sure... but at this point, I have no idea why it's there. To be safe, I ran our existing unit test suite against a version of Web with this block removed (no other changes)... and all the unit tests passed. Why is it there? Note that this approach was enough to allow for an easy repro of #12985 as well.

Not yet done - adding more unit tests to validate rules of the first form listed above; we want a clear "before and after" picture for this and any related changes.

@jahorton jahorton self-assigned this Jan 22, 2025
@jahorton jahorton added this to the A18S20 milestone Jan 22, 2025
@mcdurdin
Copy link
Member

Yes, I have also reproduced this. It is exacerbated by the #12985 bug in the compiler, because that was producing incorrect indices, masking this issue somewhat in the sil_greek_polytonic keyboard.

Looking at the commit history, it underscores the need for detailed commit comments that explain rationale. Let's do better in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants