One final oddity - this zapSelectionWithCompositionWith: seems to have 
appeared in squeak 4.1 at the same time as new editors were introduced 
on the morphic side. There is no sign of this composition business in 
the new morphic editors. I wonder if that was a design decision or just 
a fork that happened and somebody changed one fork and not the other. 
This leads me to think that ditching zapSelectionWithCompositionWith: 
would not be missed by anyone.

Cheers,
Bob

On 2/9/13 3:29 PM, David T. Lewis wrote:
> Bob,
>
> I tried reverting zapSelectionWithCompositionWith: to its original
> implementation, and making the change that you suggest to readKeyboard.
> It works exactly as you describe, including when I type rapidly in an
> image with the wait2ms delay.
>
> I will make this update to Squeak trunk. To summarize the changes
> we will have at that point:
>
> 1) The wait2ms is removed
> 2) ParagraphEditor>>readKeyboard will always process one character at a time
> 3) No change to zapSelectionWithCompositionWith:
>
> Thanks very much for your help on this!
>
> Dave
>
> On Sat, Feb 09, 2013 at 02:57:32PM -0500, Bob Arning wrote:
>> On 2/9/13 1:48 PM, David T. Lewis wrote:
>>> 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.
>> Remember that there is another "[sensor keyboardPressed] whileTrue: "
>> higher up that encompases this, so I don't think this is something to
>> worry about.
>>> 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.
>> It was that way in 1998. My guess is that if multiple input characters
>> were available, it made some sense efficiency-wise to insert them into
>> the text all at once rather than piecemeal. Back the people typed just
>> as fast as today, but the processors were much slower.
>>> 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.
>> I put your change in an otherwise unchanged 4.4 and mashed keys very
>> quickly. Everything got entered, but the cursor got farther and farther
>> behind the end of the text. This is on a Mac where 2ms seems to be 2ms.
>> I then changed wait2ms to wait 20ms and things got worse - it took long
>> pauses for characters to appear and when they did, the cursor did not
>> keep pace with the end of the text. What might be interesting to know
>> how often (if ever) your tests actually had more that one byte to
>> process in the zap methods. I think a single character works fine either
>> way, multiple characters work fine in the original zap.. but multiple
>> characters in the new zap are the problem.
>>> 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 did and that's what got me started here - I was trying to see
>> sluggish input in MVC, so I mashed keys as fast as I could and they all
>> made  it to the screen, just not always at the end of the text.
>>> 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.
>> I don't think so. That code has a purpose - the UnicodeCompositionStream
>> is used to combined multiple characters (like a letter followed by a
>> separate accent character) into a single character. It is basically
>> saying: if there are 1 or more characters in the typeahead and the first
>> is one of these combining characters (accent) and there is a character
>> in the text before the insertion point, then back up 1 space so we can
>> process that character and the accent together. I suspect it works fine
>> when there is just once character in the typeahead and that character is
>> an accent. I don't think it works well when there are more than one
>> character in the input. Hence my suggestion to do it a char at a time.
>>
>> Cheers,
>> Bob
>>> 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.
>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20130209/11b7d976/attachment.htm