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

Context opcode does not match when in a capitalized word/passage #1461

Closed
tibbsa opened this issue Oct 14, 2023 · 3 comments · Fixed by #1477
Closed

Context opcode does not match when in a capitalized word/passage #1461

tibbsa opened this issue Oct 14, 2023 · 3 comments · Fixed by #1477
Labels
bug Bug in the code (not in a table)
Milestone

Comments

@tibbsa
Copy link
Contributor

tibbsa commented Oct 14, 2023

A multi-pass "context" fails to match if the pattern occurs within a capitalized word (or passage), even though it matches fine in non-capitalized text.

I originally asked about this on the liblouis discussion list, but with no response I'm proceeding on the basis that there is in fact some bug or undocumented behaviour happening here.

Thinking there was perhaps some underlying issue within the UEB tables, I've tried to create a minimally viable example in the YAML file itself. The first 3 tests (around, Around, arOund) work as expected, with the "ound" contraction being applied in the first two cases but not the third. The last 3 examples (where the entire word or passage is capitalized) fail as the "ound" contraction does not get used at all.

display: tables/unicode.dis

table: |
  include tables/latinLetterDef6Dots.uti
  include tables/spaces.uti

  capsletter 6
  begcapsword 6-6
  lencapsphrase 3
  begcapsphrase 6-6-6
  endcapsphrase after 6-3

  always ar 345
  
  noback context _$l["ound"] @46-145a
  noback context _$l["OUND"] @46-145a

  noback pass2 [@6-46-145a]   @6-1256-1345-145
  noback pass2 [@6-6-46-145a] @6-6-1256-1345-145
  noback pass2 [@6-3-46-145a] @6-3-1256-1345-145
  noback pass2 @46-145a       @46-145

tests:
  - - "around"
    - "⠜⠨⠙"
  - - "Around"
    - "⠠⠜⠨⠙"
  - - "arOund"
    - "⠜⠠⠕⠥⠝⠙"

  - - "AROUND"
    - "⠠⠠⠜⠨⠙"
  - - "MOVE AROUND"
    - "⠠⠠⠍⠕⠧⠑⠀⠠⠠⠜⠨⠙"
  - - "GO MOVE AROUND"
    - "⠠⠠⠠⠛⠕⠀⠍⠕⠧⠑⠀⠜⠨⠙⠠⠄"

See: caps-vs-context.yaml.txt

This is blocking #1451 as I have a mostly working solution for that, but it fails for capitalized words/phrases.

@bertfrees bertfrees added the bug Bug in the code (not in a table) label Oct 17, 2023
@bertfrees
Copy link
Member

@tibbsa Thanks for the investigation and the test file, and sorry that you got no response to your message to the mailing list.

I think this might be related to #818, and the associated test file.

Both issues are caused by an inconsistency between how context rules are compiled/stored and how they are matched with input. When a particular input segment, say "OUND", is processed:

  • the first two letters "OU" are downcased
  • then, based on these two letters", the noback context _$l["ound"] @46-145a rule is selected as a candidate matching rule
  • then the _$l["ound"] test is actually performed on "OUND", returning in no match found

@bertfrees
Copy link
Member

My suggestion is to fix the second step:

  • then, based on these two letters", the noback context _$l["ound"] @46-145a rule is selected as a candidate matching rule

Both context _$l["ound"] @46-145a and context _$l["OUND"] @46-145a should be found as candidates. Moreover, they should be in the same ordered list together with other candidates. First looking for a match based on the letters "ou", and then looking for a match based on "OU" would complicate things, as you would have to compare the two to determine which rule should win, which is currently the job of the table compiler.

Making _$l["ound"] match "OUND", in other words making context rules case insensitive, would be another solution, but would be too drastic. It's not unlikely that some authors rely on the fact that context rules are case sensitive.

@bertfrees
Copy link
Member

I propose the following change:

diff --git a/liblouis/compileTranslationTable.c b/liblouis/compileTranslationTable.c
index 0338b3d3..8c98b49d 100644
--- a/liblouis/compileTranslationTable.c
+++ b/liblouis/compileTranslationTable.c
@@ -4461,6 +4461,43 @@ finalizeTable(TranslationTableHeader *table) {
 			characterOffset = character->next;
 		}
 	}
+	// Rearrange rules in `forRules' so that when iterating over candidate rules in
+	// for_selectRule(), both case-sensitive and case-insensitive rules are contained
+	// within the same ordered list
+	for (unsigned long int i = 0; i < HASHNUM; i++) {
+		TranslationTableOffset *rp = &table->forRules[i];
+		while (*rp) {
+			TranslationTableRule *r = (TranslationTableRule *)&table->ruleArea[*rp];
+			// For now only move the rules that we know are case-sensitive, namely
+			// `context' rules. (Note that there may be other case-sensitive rules that
+			// we're currently not aware of.) We don't move case insensitive rules because
+			// the user can/should define them using all lowercases.
+			if (r->opcode == CTO_Context) {
+				unsigned long int hash = _lou_stringHash(&r->charsdots[0], 1, table);
+				if (hash != i) {
+					// compute new position
+					TranslationTableOffset *rrp = &table->forRules[hash];
+					while (*rrp) {
+						TranslationTableRule *rr =
+								(TranslationTableRule *)&table->ruleArea[*rrp];
+						if (r->charslen > rr->charslen)
+							break;
+						else if (r->charslen == rr->charslen && rr->opcode == CTO_Always)
+							break;
+						rrp = &rr->charsnext;
+					}
+					// remove rule from current list and insert it at the correct position
+					// in the new list
+					TranslationTableOffset tmp = r->charsnext;
+					r->charsnext = *rrp;
+					*rrp = *rp;
+					*rp = tmp;
+					continue;
+				}
+			}
+			rp = &r->charsnext;
+		}
+	}
 	table->finalized = 1;
 	return 1;
 }

Note that it can be optimized a bit, but this gives you the idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in the code (not in a table)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants