Abdelrazak Younes wrote:
Dov Feldstern wrote:
I'm also attaching another version of it (v3), which is functionally
equivalent, but where I made the reverseDirections method a static
method of Bidi.
I committed this version but replaced the static method with a simple
helper function reverseDirectionNeeded(). Maybe we'll need to tweak this
method before 1.5.0, we'll see.
Abdel.
Thanks!
Just a few comments (sorry...):
*) I'm not sure that changesets [18351] and [18352] are correct; perhaps
I should have left cur.isRTL() in (I only removed it because nobody was
using it, I forgot to take Bernhard's patch into account).
We should just be aware of the fact that there are certainly situations
in which isRTL() is the correct function to use, and not
reverseDirectionNeeded --- they don't mean the same thing. That's why I
still left it in in Text3.cpp, and certainly isRightToLeftPar() is still
often necessary. reverseDirectionNeeded() is meant for the very specific
situation of determining if the arrow keys should be reversed.
In the context of the above changesets, I'm not sure which is the
correct one to call, but I think that it should actually be isRTL() ...
*) reverseDirectionNeeded() should probably be defined as const. I only
removed that because I had made it static. And I had only made it
static, because I didn't think that it would be recognized otherwise,
and that's why I was accessing it with Bidi::. But I guess all that is
not necessary, because of namespaces?
Anyhow, it should probably be made const.
Thanks again, and sorry for the mixups...
Dov
