Hi Bob, thanks for this.

I am hopelessly out of my depth on multilingual processing and multibyte
characters, but I'm sure others on the list will know: Are there cases
(e.g. in Japan or Korea) where keyboards provide multi-byte inputs, either
directly or through X11? If so, the "sensor keyboardPressed whileTrue:"
may be necessary in order to process those key entries. Otherwise, I
suspect you are right that it is easier to just process one character at
a time. I note also that the "whileTrue:" approach dates back to at least
2002, well before introduction of multilingual support in Squeak. I don't
know if that's significant, or if that's just the way it happened to have
been done.

Just by way of explanation, here is how I found and verified the change
to zapSelectionWithCompositionWith: in ST80-dtl.141.mcz:

1) In the original method, I forced the method to always execute the
multi-character path through the code, even if the input string was one
character long (the normal case). So I changed from this:

  ((aString isEmpty or: [(beforeChar := self charBefore) isNil]) or: [
    aString size = 1 and: [(Unicode isComposition: aString first) not]]) ifTrue: [
      ^ self zapSelectionWith: (Text string: aString emphasis: emphasisHere)].

to this:

  ((aString isEmpty or: [(beforeChar := self charBefore) isNil]) or: [
    aString size = 1 and: [(Unicode isComposition: aString first) not]]) ifTrue: [
      "^ self zapSelectionWith: (Text string: aString emphasis: emphasisHere)" ].

Result: All characters typed into an MVC workspace resulted in the cursor
positioned incorrectly to the left of the current character.

2) I changed the method to reposition the cursor correctly, leaving the
above change in place to continue forcing execution of the multi-byte code
path. This fixed the cursor positioning problem in the MVC workspace.

3) I removed the change from step 1 so that single character input would
be handled through the normal path, and verified that the MVC workspace
still worked.

4) I filed the updated method into another image that still has the wait2ms
delays, and I ran that image on a Linux interpreter VM (with "2 ms" delays
that might be more like 15 ms). Result: Cursor positioning now worked correctly
in the MVC workspace, even though keyboard entry was slow.

Conclusion: The patch in ST80-dtl.141.mcz does address a real bug in cursor
positioning that occured only in cases where sluggish key entry resulted
in multi-byte strings (presumably two characters) being handled in the
zapSelectionWithCompositionWith: method.

The problem that you identified with your "z123" test does not seem to be
occurring in practice, but it certainly looks as though it *could* occur.
I tested only by typing into a workspace from my US ascii keyboard, so I
certainly would not rule out the possibility of further problems.

I do not know if it is safe to make the change you suggest to process one
character at a time. It looks to me like it would be good to do that, but
I don't know if it might cause problems for non-US keyboards or locales.

If it was safe to make that change, then the entire bit of code for handling
multi-byte strings in zapSelectionWithCompositionWith: would become irrelevant,
and we could just get rid of it.

Hopefully someone with experience in this area can help?

Dave


On Sat, Feb 09, 2013 at 06:52:43AM -0500, Bob Arning wrote:
> I fear that doesn't even work with ordinary USASCII input. I tweaked 
> readKeyboard to simulate additional characters arriving in close succession:
> 
>         self hasSelection ifTrue: "save highlighted characters"
>             [UndoSelection := self selection].
> *((model isKindOf: Workspace) and: [char = $z]) ifTrue: [typeAhead 
> nextPutAll: '123'].*
> 
>         self zapSelectionWithCompositionWith: typeAhead contents.
> 
> and what I get by typing some z's in a Workspace is:
> 
> z123zzzzzz123123123123123123
> 
> I think the safest way to guard against multiple characters arriving in 
> quick succession is to simply process each one completely before 
> proceeding to the next. This, after all, is the likeliest case by far 
> for modern computers and ordinary humans typing:
> 
>         self deselect.
> *sensor keyboardPressed ifTrue: "was a whileTrue:"*
>             [char := sensor keyboardPeek.
>             (self dispatchOnCharacter: char with: typeAhead) ifTrue:
>                 [self doneTyping.
>                 self setEmphasisHere.
>                 ^self selectAndScroll; updateMarker].
>             self openTypeIn].
> 
> Cheers,
> Bob
> 
> On 2/9/13 1:01 AM, commits at source.squeak.org wrote:
> >aText := Text string: newString emphasis: emphasisHere.
> >- 	self markBlock < self pointBlock
> >- 		ifTrue: [self setMark: self markBlock stringIndex - 1]
> >- 		ifFalse: [self setPoint: self  pointBlock stringIndex - 1].
> >-
> >   	wasComposition := true.
> >+ 	self markBlock < self pointBlock
> >+ 		ifTrue: [ self setMark: self markBlock stringIndex - 1.
> >+ 				self zapSelectionWith: aText.
> >+ 				self setMark: self markBlock stringIndex + 1]
> >+ 		ifFalse: [ self setPoint: self pointBlock stringIndex - 1.
> >+ 				self zapSelectionWith: aText.
> >+ 				self setPoint: self pointBlock stringIndex + 
> >1]
> >- 	self zapSelectionWith: aText.
> 

>