See also discussion on mailing list

I like these changes but I think they need to be fixed as @bertfrees says in the discussion on the mailing list. Moved to next release

Yep.

Hello Christian,

I hope you can add my changes to logging.c. This was accidentally added to the same pull request as Bert Frees knows.

@egli: Arend is referring to PR #142

Basically commit ArendArends@1a63a3d needs to be cherry-picked

OK, I cherry picked the logging changes. They will be in the release.

@ArendArends @bertfrees just told me that if you provide a test for the problem that the ouptput positions aren't correct then he would fix the problem in the right place.

@ArendArends would you mind providing a test for the problem that the ouptput positions aren't correct?

Thanks

@jcsteh Is this why NVDA translates with pass1Only? I don't pretend to understand the wizardry going on in this code, but presumably it needs more work.

@dkager commented on Jun 5, 2017, 10:12 PM GMT+10:

@jcsteh Is this why NVDA translates with pass1Only?

Yes. Also, back when we started using pass1Only, the input positions array was also broken for multi-pass. That has since been fixed, though I never got around to testing/verifying that myself.

@ArendArends, are you willing to take action on this? Not being able to use multi pass reliable in NVDA has some serious drawbacks.

Since this pr looks a bit abandoned, is anyone willing to fork it and review the changes? I'm not very acquainted to liblouis code, but I'm happy to give a try, for example if @dkager doesn't have resources to spend on this.

I'm willing to have a look if I have some good test data.

I'd like to have some outputPositions and cursorPosition tests for various multipass rules. Because NVDA seems to be the only one who is using these features I think they now best what the expected behavior is and what currently goes wrong, but any help is appreciated. If I can just get a description of how the cursorPosition and outputPositions should behave for the basic operations that would already help:

• replacements: mapping of characters to dot patterns, contractions, ...
• insertions: e.g. various braille indicators
• deletions

Also some examples of what currently goes wrong would be nice.

To ensure that the cursor never goes out of bounds sounds like an absolute minimum. The goal is to make the cursor as accurate as possible.

Note that the expected behavior of inputPositions is clear (I'm using that feature myself) however some additional tests for it wouldn't hurt either.

Ok, here are some thoughts on the expected behavior:

Cool, thanks!

As far as I have tested it, the cursor positions seem to make sense for pass 1. For word contractions, the cursor can only be in the first position of the word, since the word is “atomic” as liblouis is concerned. On the display, you can’t move the cursor to the b in “abv” (above).

In all partword contractions (begword, midword, always etc.), The cursor can only be in the first position of the contraction, e.g. position 1, 3 and 4 in “youngster” (5-13456-34-12456).

In a way it is reasonable that Liblouis treats all these replacement rules as atomic, if you assume that rules can be written in such a way that they represent single contractions only.

I know that there are cases where this assumption can not be made, namely when more context is needed. To answer this problem we could add special opcodes to provide such context (I believe LiblouisAPH has a nice way to do this). Or, we can rely on hyphenation patterns to provide the context, via the "nocross" feature.

Another thing we should ask ourselves is whether contractions are by definition atomic or not? In your "youngster" example, the expected behavior is clear I think, namely that this word consists of 3 contractions, and this also matches the current behavior of liblouis:

young, st, er -> "y, /, }

Your "above" example is less clear: should the "b" in "abv" be mapped to the "b" in "above" or not? And what the "v"? You could argue that there are really two contractions: bo -> b and ve -> v.

In the case of indicators, the cursor is apparently never on the indicator, but always on the following character. One could argue against this practice that in the cases where the indicator is created in liblouis as part of the character itself, e.g. accent indicator followed by a letter, then the cursor will actually be on the indicator and not the letter. The same goes for letters with letsign as part of their definition.

OK, that makes sense. At least for letsign it does. I'm not sure what you mean by "accent indicator". If an accented letter needs a special indicator this is usually implemented with a multi-cell character definition, which results in the expected behavior.

With multi-pass rules, I think it could be very simple, as all operations are essentially search and replace operations.

If the cursor is after or at the position where replacement happens, the cursor should be moved forward or back according to the number of characters or cells added or subtracted. We cannot know the nature of what is being replaced with what, e.g. if it is indicators or contractions, but I think it is the best we can do.

For simple replacements it is indeed straightforward. But actually a multi-pass rule can also result in an insertion or a removal, in which case there are two possibilities for the output and input positions arrays respectively. In these cases we just have to choose one of the two possibilities, because we can not know the nature of the insertions or removals.

nocross ger 1245-156

Why don't you just change this into nocross er 156 and make sure there is a break point at g-er, which inhibits the ge 12456 rule?

In the “above” example, however, “above” would probably be considered atomic according to the Braille rules. Removing the v gives quite a different word “about”.

I agree that it probably makes more sense to treat this as atomic. However when I suggested that this could be split up into the smaller contractions "bo" and "ve" I was thinking about them as optional contractions, just like the "ge" contraction in Danish is optional. In "about", the "bo" contraction looses from the "about" contraction.

Could you please give me an example of insertion or deletion where the resulting cursor position is unclear?

In your example, "foo-bar" would be translated to "foo---bar". It is clear that the position between "o" and "-" in the input maps to the position between "o" and "-" in the output, and that the position between "-" and "b" maps to the position between "-" and "b" in the output. However when "foobar" is translated to "foo-bar" there are two possible output positions that the input position between "o" and "b" can map to: either between "o" and "-" or between "-" and "b".

It's exactly the same issue as with the insertion of braille indicators. Whether the cursor should be before or after the indicator depends on the type of indicator.

With deletions there is a similar ambiguity, but this time in the input positions instead of the output positions.

OK thanks!

I'm not entirely sure what the YAML framework currently can and can't do. If needed we should definitely improve it. @egli do you want to answer this?

As far as I remember the YAML test tool basically just exposes the functionality of brl_checks.c, so only cursor position no input or output positions. Looking at the code of brl_checks.c though it appears that input and output position tests have been implemented. So it would be possible to enhance lou_checkyaml.c to support those tests. Just come up with a suitable YAML notation and hook it up to the code in brl_checks.c (and document it, of course).

If I understand correctly when you do the following:

cursor_pos = cursor_pos_in;
lou_translate(table, inbuf, &inlen, outbuf, &outlen, NULL, NULL,
              outpos, inpos, &cursor_pos, mode));

the expression outpos[cursor_pos_in] == cursor_pos is always true, regardless of the mode. In that sense there is a bit of redundancy in the cursor API. The resulting translation and resulting cursor position may of course differ when the mode changes, but for each inbuf/cursor_pos/mode combination, outpos and cursor_pos before and after always correspond.

Note that there is more redundancy in the API, namely between inpos and outpos. With the current definitions of inpos and outpos, one can (almost) always be computed from the other.

In the YAML tests the cursorPos option tests the resulting cursor positions in the output for each of the possible cursor positions in the input, with mode set to compbrlAtCursor. If you would do this without the compbrlAtCursor mode, you will indeed get the same result as when you would just take the resulting output positions array of a single translation.

The example in the documentation doesn't make much sense by the way. In the actual test where it was taken from is also marked with xfail.

-  - you went to
    - ⠽ ⠺⠑⠝⠞ ⠞⠕
    - mode: [compbrlAtCursor]
      cursorPos: [0,1,2,3,4,5,6,7,8,9,10]

One thing about the cursorPos tests just doesn't seem right to me. When cursorPos is present the expected translation isn't checked at all. But the single expected translation also doesn't make sense. If you really want to test with a cursor at all possible positions, you need a list of the same number of expected translations. Testing an index without knowing what it is the index points to just doesn't make much sense to me.

Maybe it is easier to just forget about testing all cursor positions at once, and instead just repeat the same test with as many different cursor positions as desired. cursorPos would then just accept a single int like it was before in the JSON tests.

Another thing I discovered while looking at the code of lou_checkyaml.c is that the specified mode isn't even ever used. When cursorPos is present it simply always tests with "compbrlAtCursor" mode. When cursorPos is not present it tests with the default mode (0). This is probably just an oversight.

Regarding a good syntax for inputPos and outputPos tests: we could of course do it with an array of ints, like we are currently doing with cursorPos, however that is not very readable at all, so I think we should try to come up with something more clever.

How about some thing like this:

- - "you went to" # input
  - "⠽⠽⠽ ⠺⠑⠝⠞ ⠞⠕" # corresponding character in braille for each character in input
  - "⠽ ⠺⠑⠝⠞ ⠞⠕"   # braille
  - "y went to"   # corresponding character in input for each character in braille

Note that if you don't have a true monospace font (Github doesn't) this doesn't really have the desired effect.

Of course this is not fully unambiguous when you have test cases with repeating characters. We could also do it like the next example, it's slightly more readable but it adds even more ambiguity, namely in the spaces:

- - "you went to" # input
  - "⠽   ⠺⠑⠝⠞ ⠞⠕" # corresponding character in braille for each character in input
                  # but without repeating braille characters
  - "⠽ ⠺⠑⠝⠞ ⠞⠕"   # braille
  - "y went to"   # corresponding character in input for each character in braille
                  # but without repeating input characters

The spaces that are used instead of repeated characters could possibly be replaced with underscores or another type of spaces.

A completely different approach would be to indicate all the possible inter-character positions in the input and in the braille that correspond with each other. This resembles how I always like to visualize the mapping, namely by placing the input and output strings above each other and connecting the corresponding character boundaries in text and braille with lines. In a simple text format you can not quite do that, but this comes close. In this example I'm using vertical bars to indicate the positions:

- - "|you| |w|e|n|t| |t|o|"
  - "|⠽| |⠺|⠑|⠝|⠞| |⠞|⠕|"

Note that with the current definitions of inpos and outpos you can not do things like this:

- - "|foo|removal|bar|"
  - "|⠋||⠃|"

For typical applications this kind of information would be useless anyway. I imagine a cursor on a braille display always takes up one cell, and can't suddenly take on another form when you move it into the segment of the word that was removed. (To understand what I mean with "take on another form", try inserting in Emacs: "a", "b", <ZWSP>, "c" (A B ^X 8 RET 2 0 0 B RET C) and move the cursor through the word.) For more specialized applications, this kind of information could possibly be marginally useful. However I don't even know of any real examples of the kind of translation shown above. So I think we should worry about it at the moment.

This also brings us back to the original goal of this issue: to fix the output positions. Given the (almost complete) redundancy between inpos and outpos I think the easiest way to fix this is to compute outpos from inpos (or more correctly: compute both of them from a common variable) all the way at the end of the translation.

[...] it would be nice to also have the option of a numerical output like the current one for cursorpos.

Well, having the choice doesn't really help because then you're still in the same situation if you want to read a test from someone else who chose the "wrong" syntax. There should be only one syntax I think.

But, I'm trying to understand... Why exactly would my syntax be more difficult to read with speech than the one with numbers? Can you explain? Note that I'm only using vertical bars as an example to explain the principle. There are various other ways to get the desired effect. And what about reading it with braille display?

Ok, I thought the problem was maybe something else.

Yes, of course, I understand the dimension issue. But my proposed syntax doesn't need two dimensions to be useful. You can simply check whether the segmentation of the input and the output makes sense, one at a time. A very natural thing. Of course two dimensions makes it even easier, but it is not essential.

Parsing arrays of numbers in your head is more difficult in my opinion, especially in one dimension. When you are checking input positions for example, you need to keep track of where you currently are in the output string, but at the same time you need to count characters in the input string in your head. Too much thinking.

I agree that in an ideal world we would be separating data and presentation, but if we can find a presentation that is easy to read for everyone I think that's preferable.

Just wanted to point out that I filed nvaccess/nvda#7702, which allows NVDA users to disable the pass1only flag. This might give more information about the what, when and how of things going wrong.

Before adding more complexity, I would first like to try if people can achieve the desired cursor behavior by rewriting their multipass rules if needed.

For example, the following rules do the same thing but result in a different cursor mapping:

correct "foo"["-"]"bar" ?
correct "fo"["o-"]"bar" "o"

I'm going to close this now. The tests are being worked on in #430.

I"m not exactly sure whether I understand the current status of this. Are there still noticeable bugs which will be covered by #430?

Yes the bug hasn't been fixed. See the tests at https://github.com/BueVest/liblouis/blob/222dd44eb695c118dc9940c4ec0b8759b43680de/tests/yaml/inpos_outpos.yaml marked with xfail. I'm currently working on it and it will be included in release 3.4.