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

Change precedence of character definition and base rules #1481

Merged
merged 1 commit into from Dec 4, 2023

Conversation

bertfrees
Copy link
Member

Character definition and base rules are now considered in the order in which they are defined in the table.

@bertfrees bertfrees requested a review from egli November 29, 2023 12:52
@bertfrees bertfrees added this to the 3.28 milestone Nov 29, 2023
@bertfrees bertfrees added enhancement An enhancement in the functionality (not a bug fix or a table improvement) needs news Update to NEWS file needed cleanup Cleanup (possibly removing features) to make the code more maintainable and removed cleanup Cleanup (possibly removing features) to make the code more maintainable labels Dec 1, 2023
@rimas-kudelis
Copy link
Contributor

So the new precedence is that the topmost rule wins?
I find it a bit unusual, to be honest. If this is still open for discussion, I'd vote for the last rule always winning ("always" meaning it should consistently apply to any rule that can be overridden). I think this is more common in programming, at least from my experience.

@bertfrees
Copy link
Member Author

That's right, the topmost rule wins. It may be a bit unusual, but it was decided to do it like that:

  1. because it is most in line with how Liblouis currently works. For example, translation rules with the same length are also considered in the order in which they are defined. Changing it the other way would have been a much bigger (backward incompatible change) and there would have to be a lot more modifications to the tables.

  2. because of some technical challenges to do it the other way around.

@rimas-kudelis
Copy link
Contributor

Fair enough. I thought this might have been the case with other rules...

Just one other thing then: the phrase "rules are considered in the order in which they are defined" is still not clear, at least to me. "Topmost rule wins" makes it perfectly explicit though, so perhaps that or similar clarification phrase should be added to all explanations.

Character definition and base rules are now considered in the order in
which they are defined in the table.
@egli egli force-pushed the character-definition-precedence branch from 6984d58 to e6f842d Compare December 4, 2023 11:10
@egli egli merged commit 322452c into master Dec 4, 2023
12 checks passed
@egli egli deleted the character-definition-precedence branch December 4, 2023 11:21
@egli egli removed the needs news Update to NEWS file needed label Dec 4, 2023
@hammera
Copy link
Contributor

hammera commented Dec 4, 2023

Bert, Chris, I readed following part the NEWS file:

** Backwards incompatible changes
- The order of how character definition and base rules are considered
  has changed. All included tables were adapted, but if you maintain
  your own tables you might want to look [[https://github.com/liblouis/liblouis/pull/1481][at the change]] to see what you
  need to do.

I looked entire merged diff output, normal yaml tests are right passed with hungarian language.
In hungarian tables related only tables/hu-chardefs.cti file are use base opcode, but both hu-hu-g1.ctb and hu-hu-g2.ctb tables are include this subtable file.
So, me need changing anything future the 3.29.0 development cicle the tables/hu-chardefs.cti file, or enough to future will be running my local machine a large corpus test only with examine forward and backward translation in grade 1 the bothDirection flag?
The manual and dictionary based hungarian yaml tests with awailable the online check system not showed any problems with affects the hungarian tables related, so this change are I thing not will be produce future problems in the hungarian tables.
Congratulate the 3.28.0 release.

Attila

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement in the functionality (not a bug fix or a table improvement)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants