Daniel Vainsencher <danielv at netvision.net.il> wrote:
(in response to a contribution about Text protocol)

> [everything else about Text operations]
> Sorry, but I've run out of steam. All that ugly code is depressing me.
When I received this message, I agreed half-way. This evening,
after half a day of fighting against a Text problem, I agree fully.

To tell you the story:

Text>>
replaceFrom: start to: stop with: replacement startingAt: repStart 
 "This destructively replaces elements from start to stop in the
   receiver starting at index, repStart, in replacementCollection.
   Do it to both the string and the runs."

 | rep newRepRuns |
    rep _ replacement asText. "might be a string"
   string replaceFrom: start to: stop with: rep string startingAt: repStart.
   repStart = 1 
     ifTrue: [runs _ runs copyReplaceFrom: start to: stop with: rep runs]
     ifFalse:
      [newRepRuns _ rep runs copyFrom: repStart to: rep size.
       runs _ runs copyReplaceFrom: start to: stop with: newRepRuns].

is wrong, but it works for an important special case.
Have a look at newRepRuns :
It is assigned a collection that contains the text attributes of the
replacement text from the first character to be copied into the receiver
up to the end of the replacement string. This works when 
rep size - repStart + 1 (the number of characters to be copied from
rep  into the receiver) is equal to  stop - start + 1 (the number of
charcters that are replaced in the receiver.)

I think the method should read: 

replaceFrom: start to: stop with: replacement startingAt: repStart 
 "This destructively replaces elements from start to stop
 in the receiver starting at index, repStart, in replacement
Collection. Do it to both the string and the runs."

 | rep newRepRuns |
    rep _ replacement asText. "might be a string"
   string replaceFrom: start to: stop with: rep string startingAt: repStart.
   repStart = 1 
     ifTrue: [runs _ runs copyReplaceFrom: start
                                  to: stop
                                  with: (rep runs copyFrom: 1 to: stop - start + 1)]
    ifFalse: 
     [newRepRuns _ rep runs copyFrom: repStart to: repStart + stop - start.
      runs _ runs copyReplaceFrom: start to: stop with: newRepRuns].

Here, I assign to newRepRuns  a collection that contains the
text attributes of the replacement text for the characters that are 
copied into the receiver.
My proposal is still not perfect, one should verify that 

rep size >= repStart + stop - start

 (repStart + stop - start  is the index of the last character that is
copied from  rep into the receiver. )

> How would you feel about doing some groundwork on those classes and
> their clients, and then writing some tests that check the
> Text/TextStream API? this would be a good prelude to the kind of deep
> cleanup these classes are begging for (IMHO).
I cannot deny that we should have tests. We should also have some
half-formal description of the classes RunArray and Text. Both
classes are based on really excellent ideas, but we do not care to explain
these ideas.
I will try to write some tests, but this will take a bit more time. Perhaps
I can also discuss this stuff at CampSmalltalk - a serious CampSmalltalk
should have a Squeak bug fixing party. I will try to publish a first set
of tests before CampSmalltalk.
> 
> Daniel
Boris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20030610/8652d2d7/attachment.htm