Various improvements to emphasis handling #1006
Conversation
bertfrees
added
enhancement
|
@josteinaj I changed some tests for Norwegian (not the table). Would you mind having a look at https://github.com/liblouis/liblouis/pull/1006/files#diff-531a3730363a4ab7232b5933f0e0a94668637586bdf712cd740307062cdc704f? |
|
@garconvacher I also did a small change to the French table: see commit b4604c1. Could you verify please? |
|
@dkager Some Dutch tests were fixed but one was broken. To me the new result makes sense. What do you think? liblouis/tests/braille-specs/nl-g0_harness.yaml Lines 207 to 209 in ab4f0fe |
|
@torchtrust I found a strange test that was added by you in 2015: The dots 6-1-6 at the beginning, is that correct? |
bertfrees
force-pushed
the
various-emphasis
branch
from
b4604c1
to
e15019a
Compare
|
@bertfrees By just skimming through the changes in no_typeform_harness.yaml I think it looks good, but I'll check with Kari to be sure. |
@bertfrees, I can't do tests at the moment but it's match the french braille code. |
|
@dkager What do you think about the "Zie refertes D-NE004W, D-NB007B en D-NB007BDK." test? "D-NE004W, D-NB007B" is now seen as 6 words (so that phrase indication is used): "D", "NE", "W", "D", "NB" and "B". |
|
@egli Please review. It's a really big improvement, if I may say so myself. |
There was a problem hiding this comment.
Looks like a good change. Afaikt you are replacing the binary way of handling emphasis with a more flexible system.
I didn't really check if all the index bounds are kept in all corner cases.
But all in all looks very good
I think we should have some doc though
Not sure what you mean by binary, but there is still a strict distinction between what we call "permanent" indication on the one hand, and word/phrase level indication on the other hand. Very little has changed for permanent indication.
Yes, I have to add some. But note that apart from the no |
|
Sorry for the delays! Regarding this test:
https://github.com/liblouis/liblouis/blob/ab4f0fee3329a1ce5c97738f83a90ab5ec436509/tests/braille-specs/nl-g0_harness.yaml#L207-L209
I understand the description in the xfail, but think it would be good to ask Dorine in ‘t Veld (Dedicon) or another 6-dot braille expert from the Braille Autoriteit working group.
|
|
@bertfrees: Kari approves: snaekobbi#10 (comment) |
bertfrees
force-pushed
the
various-emphasis
branch
2 times, most recently
from
100f5b8
to
7d6860d
Compare
|
I've sent Dorine a message on Monday. I think whatever the outcome, I'm going to merge this PR because I think overall the Dutch table is definitely improved. Several issues where fixed, only this one border case is possibly (not certainly) a regression. |
|
Sounds sane.
|
bertfrees
removed
the
needs doc
|
@egli I have updated the doc. Can you quickly check again? |
bertfrees
force-pushed
the
various-emphasis
branch
from
aaa4c7a
to
6039326
Compare
…me before/after space
In Dutch, every word part in a capitalised compound word counts in the length of a passage. see #779
so remove begcaps. Also, endcaps is automatically used as endcapsphrase (after), but rename it anyway.
fixes commit a9f7df1
Added a new resolveEmphasisBeginEnd function that is used by both capitalization and emphasis. This is mostly refactoring, but there is also a small behavioral change, namely: - An emphasized section indicated with begemph/endemph will not start or end with a space. The idea is to make capitalization and emphasis handling more and more alike in the future.
…ation For begemphphrase it is only enabled when emphmodechars is declared.
…sis handling This changes the behavior a very little bit for tables that do not declare capsmodechars or emphmodechars, and a bit more substantially for tables that do. Tables and tests have been updated accordingly. The code has also been restructured in a way that makes it easier to see how "caps" differs from "emph" and how the various character attributes and the capsmodechars, emphmodechars and noemphchars rules affect the behavior. Extracting out this logic into a few simple functions made both that logic and the rest of the code easier to follow. See #905
This further harmonizes the behavior and further simplifies the code. The new noemphchars can be used to achieve the old behavior. Related to #1002.
- use opcoderef only when it makes sense. Where they are used with a redundant "opocde" remove that. - try to move the references out of sentences, so it is easier to read - try to group references
There is a macro to generate the text "foo opcode" with a reference. Quite often though you want to have a reference to an opcode without the preceding "foo opcode" text. For that there are two new macros `opref` and `pxopref`. They are just like `ref` and `pxref` but they make sure the reference is correct (by making it "foo opcode") and they render the opcode inside a code
bertfrees
force-pushed
the
various-emphasis
branch
from
0a89b34
to
8a08c08
Compare
This contains quite a lot. Best look at the commit logs to see what exactly. In short, I added a new opcode called
noemphchars, improved the code readability, and harmonized the behavior of caps vs. emph on the one hand and begemph/endemph mode vs. begendword etc. on the other hand.Fixes the following issues:
Issues with emphasis in German when quotation mark or full-stop precedes/follows. #998Only thing missing is a little update to the documentation.
Travis status doesn't show up anymore apparently but tests pass on Travis.