[liblouis-liblouisxml] Re: Regression in 1.3.9

  • From: "John J. Boyer" <johnjboyer@xxxxxxxxxxxxx>
  • To: liblouis-liblouisxml@xxxxxxxxxxxxx
  • Date: Sun, 18 Jan 2009 18:58:55 -0600

James, 

This switch statement is coded rather oddly. It's a holdover from
brltty. Since I wrote the original code for brltty, I probably have to
accept part of the blame. What is happening is that transRule,
transOpcode and transCharslen have already been set. If the conditional
i a case is true, for_selectRule terminates with a return, which means
that the settings for transOpcode, etc. are correct. If the conditional
is false, the case breaks, which causes the function to check whether
there are other rules that might apply at this point. So we do want the
function to continue.  Setting transOpcode to CTO_None will trigger code
for handling undefined characters. 

How are you testing? If you comment out the repeated rules in the 
translation table, do you still have problems with input and output 
mapping?

Thanks,
John

On Mon, Jan 19, 2009 at 10:33:32AM +1000, James Teh wrote:
> Hi John,
> 
> Regarding your fix in 1.3.9, you said:
> >I thought the best way to
> >handle the repeated opcode if the cursor fell in a repeated section was
> >not to execute it
> This makes perfect sense to me. However, this doesn't seem to be happening.
> 
> You added a condition which does a "break" instead of a "return" if the 
> repeated section lies between compbrlStart and compbrlEnd:
> >               case CTO_Repeated:
> >                 if ((mode & compbrlAtCursor) && src >= compbrlStart
> >                     && src <= compbrlEnd)
> >                   break;
> >                 return;
> I don't follow how breaking instead of returning causes this not to be 
> executed, as it will just move on to the next part of the function. This 
> piece of code seems to be causing problems with the input/output 
> position maps as well. Would setting transOpcode to CTO_NONE be the 
> correct solution? I'm having trouble understanding the purpose of this 
> first switch statement in for_selectRule().
> 
> Thanks.
> 
> On 16/01/2009 9:38 PM, James Teh wrote:
> >Hi John,
> >
> >I submitted a patch which you included in 1.3.9. The patch did the
> >following:
> >>When handling input which is removed
> >>due to a "repeated" opcode, correctly update the mapping from input to
> >>output positions; i.e. the outputPos array and the returned cursorPos.
> >Unfortunately, as explained in this thread:
> >//www.freelists.org/post/liblouis-liblouisxml/patch-Correct-input-to-output-position-mapping-for-removed-repeated-text,3
> >
> >it caused a rather nasty regression. I did come up with an alternative
> >to this patch, but I'm not sure that was the right solution either.
> >
> >You noted that you had a more correct solution for this which you
> >applied in 1.4.0. However, I noticed that you didn't remove my existing
> >patch from 1.3.9. Furthermore, that still doesn't seem to fix the problem.
> >
> >Do you have any further ideas on this one? I can try to provide more
> >test cases or explanation if needed. It can be a rather frustrating bug
> >for NVDA users.
> >
> >Thanks!
> >
> 
> -- 
> James Teh
> Email/MSN Messenger/Jabber: jamie@xxxxxxxxxxx
> Web site: http://www.jantrid.net/
> For a description of the software and to download it go to
> http://www.jjb-software.com

-- 
My websites:
http://www.godtouches.org
http://www.jjb-software.com
Location: Madison, WI, USA

For a description of the software and to download it go to
http://www.jjb-software.com

Other related posts: