selectionForDoitAsStream smells

> Compare this which is like old squeak selectionAsStream
>
> SmalltalkEditor>>selectionForDoitAsStream
> ^ ReadWriteStream
> on: self string
> from: self startIndex
> to: self stopIndex - 1
>
> and this which has been changed conforming to Cuis
>
> TextEditor>>selectionAsStream
> "Answer a ReadStream on the text in the paragraph that is currently
> selected."
>
> ^ReadStream
> on: (self string copyFrom: self startIndex to: self stopIndex - 1)
>
> (or self selection asString readStream)
> The two differ.
> The former stream position starts at editor startIndex - 1, while the
> later stream position starts at 0.
> The stream position is used by the Compiler/Parser for notifying the
> SmalltalkEditor (or TextMorph) requestor.
>
> Now look at SmalltalkEditor>>notify: aString at: anInteger in: aStream
> "The compilation of text failed. The syntax error is noted as the argument,
> aString. Insert it in the text at starting character position anInteger."
>
> | pos |
> pos := self selectionInterval notEmpty
> ifTrue: [self startIndex + anInteger - 1 ]
> ifFalse: [anInteger].
> self insertAndSelect: aString at: (pos max: 1)
>
> You see that it adds (startIndex - 1) to the error message insertion
> position which is a Cuis change.
> (The old ParagraphEditor did not).
>
> When #notify:at:in: is used for evaluating #selectionAsStream, all is
> OK, the (startIndex - 1) is added only once.
> But when it is used with #selectionForDoitAsStream, the offset is
> counted twice (because already in the stream location passed in
> anInteger parameter).
>
> I guess the hack #selectionForDoitAsStream that undo the Cuis clean-up
> has been added because some other part of Parser/Encoder/Compiler is
> messing with the requestor selection (like highlighting some part of
> the source for user interaction in case of ambiguity).
> Tthis is not a valid workaround.
>
> In Squeak, I did revert the Cuis clean-up just to keep some simple
> compatibility with ParagraphEditor, but Pharo does not care about
> that, so it should apply full simplification.
> I suggest removing #selectionForDoitAsStream, and replace senders with
> #selectionAsStream.
> Then if there are some other feature broken we'll fix'em.
> OK?

>
> On Feb 26, 2012, at 9:41 PM, Nicolas Cellier wrote:
>
>> Compare this which is like old squeak selectionAsStream
>>
>> SmalltalkEditor>>selectionForDoitAsStream
>>       ^ ReadWriteStream
>>               on: self string
>>               from: self startIndex
>>               to: self stopIndex - 1
>>
>> and this which has been changed conforming to Cuis
>>
>> TextEditor>>selectionAsStream
>>       "Answer a ReadStream on the text in the paragraph that is currently
>>       selected."
>>
>>       ^ReadStream
>>               on: (self string copyFrom: self startIndex to: self stopIndex - 1)
>>
>> (or self selection asString readStream)
>> The two differ.
>> The former stream position starts at editor startIndex - 1, while the
>> later stream position starts at 0.
>> The stream position is used by the Compiler/Parser for notifying the
>> SmalltalkEditor (or TextMorph) requestor.
>>
>> Now look at SmalltalkEditor>>notify: aString at: anInteger in: aStream
>>       "The compilation of text failed. The syntax error is noted as the argument,
>>       aString. Insert it in the text at starting character position anInteger."
>>
>>       | pos |
>>       pos := self selectionInterval notEmpty
>>               ifTrue: [self startIndex + anInteger - 1 ]
>>               ifFalse: [anInteger].
>>       self insertAndSelect: aString at: (pos max: 1)
>>
>> You see that it adds (startIndex - 1) to the error message insertion
>> position which is a Cuis change.
>> (The old ParagraphEditor did not).
>>
>> When #notify:at:in: is used for evaluating #selectionAsStream, all is
>> OK, the (startIndex - 1) is added only once.
>> But when it is used with #selectionForDoitAsStream, the offset is
>> counted twice (because already in the stream location passed in
>> anInteger parameter).
>>
>> I guess the hack #selectionForDoitAsStream that undo the Cuis clean-up
>> has been added because some other part of Parser/Encoder/Compiler is
>> messing with the requestor selection (like highlighting some part of
>> the source for user interaction in case of ambiguity).
>> Tthis is not a valid workaround.
>>
>> In Squeak, I did revert the Cuis clean-up just to keep some simple
>> compatibility with ParagraphEditor, but Pharo does not care about
>> that, so it should apply full simplification.
>> I suggest removing #selectionForDoitAsStream, and replace senders with
>> #selectionAsStream.
>> Then if there are some other feature broken we'll fix'em.
>> OK?
>
> Sure!!!
>
>>
>> Nicolas
>>
>
>

> One of the area of high interaction is
> Parser>>correctSelector:wordIntervals:exprInterval:ifAbort: which has
> to then to deal with (self startIndex - 1) offset.
> It will have to be fixed too.
>
> Nicolas
>
> Le 26 février 2012 21:54, Stéphane Ducasse <[hidden email]> a écrit :
>>
>> On Feb 26, 2012, at 9:41 PM, Nicolas Cellier wrote:
>>
>>> Compare this which is like old squeak selectionAsStream
>>>
>>> SmalltalkEditor>>selectionForDoitAsStream
>>>       ^ ReadWriteStream
>>>               on: self string
>>>               from: self startIndex
>>>               to: self stopIndex - 1
>>>
>>> and this which has been changed conforming to Cuis
>>>
>>> TextEditor>>selectionAsStream
>>>       "Answer a ReadStream on the text in the paragraph that is currently
>>>       selected."
>>>
>>>       ^ReadStream
>>>               on: (self string copyFrom: self startIndex to: self stopIndex - 1)
>>>
>>> (or self selection asString readStream)
>>> The two differ.
>>> The former stream position starts at editor startIndex - 1, while the
>>> later stream position starts at 0.
>>> The stream position is used by the Compiler/Parser for notifying the
>>> SmalltalkEditor (or TextMorph) requestor.
>>>
>>> Now look at SmalltalkEditor>>notify: aString at: anInteger in: aStream
>>>       "The compilation of text failed. The syntax error is noted as the argument,
>>>       aString. Insert it in the text at starting character position anInteger."
>>>
>>>       | pos |
>>>       pos := self selectionInterval notEmpty
>>>               ifTrue: [self startIndex + anInteger - 1 ]
>>>               ifFalse: [anInteger].
>>>       self insertAndSelect: aString at: (pos max: 1)
>>>
>>> You see that it adds (startIndex - 1) to the error message insertion
>>> position which is a Cuis change.
>>> (The old ParagraphEditor did not).
>>>
>>> When #notify:at:in: is used for evaluating #selectionAsStream, all is
>>> OK, the (startIndex - 1) is added only once.
>>> But when it is used with #selectionForDoitAsStream, the offset is
>>> counted twice (because already in the stream location passed in
>>> anInteger parameter).
>>>
>>> I guess the hack #selectionForDoitAsStream that undo the Cuis clean-up
>>> has been added because some other part of Parser/Encoder/Compiler is
>>> messing with the requestor selection (like highlighting some part of
>>> the source for user interaction in case of ambiguity).
>>> Tthis is not a valid workaround.
>>>
>>> In Squeak, I did revert the Cuis clean-up just to keep some simple
>>> compatibility with ParagraphEditor, but Pharo does not care about
>>> that, so it should apply full simplification.
>>> I suggest removing #selectionForDoitAsStream, and replace senders with
>>> #selectionAsStream.
>>> Then if there are some other feature broken we'll fix'em.
>>> OK?
>>
>> Sure!!!
>>
>>>
>>> Nicolas
>>>
>>
>>

> http://code.google.com/p/pharo/issues/detail?id=5406
>
> SmalltalkEditor>>notify:at:in: is wrong and has been reverted in Cuis too.
>
> I have a plan explained in above issue if we want to get rid of
> ReadStream on:from:to:
>
> Nicolas
>
> Le 26 février 2012 21:57, Nicolas Cellier
> <[hidden email]> a écrit :
>> One of the area of high interaction is
>> Parser>>correctSelector:wordIntervals:exprInterval:ifAbort: which has
>> to then to deal with (self startIndex - 1) offset.
>> It will have to be fixed too.
>>
>> Nicolas
>>
>> Le 26 février 2012 21:54, Stéphane Ducasse <[hidden email]> a écrit :
>>>
>>> On Feb 26, 2012, at 9:41 PM, Nicolas Cellier wrote:
>>>
>>>> Compare this which is like old squeak selectionAsStream
>>>>
>>>> SmalltalkEditor>>selectionForDoitAsStream
>>>>       ^ ReadWriteStream
>>>>               on: self string
>>>>               from: self startIndex
>>>>               to: self stopIndex - 1
>>>>
>>>> and this which has been changed conforming to Cuis
>>>>
>>>> TextEditor>>selectionAsStream
>>>>       "Answer a ReadStream on the text in the paragraph that is currently
>>>>       selected."
>>>>
>>>>       ^ReadStream
>>>>               on: (self string copyFrom: self startIndex to: self stopIndex - 1)
>>>>
>>>> (or self selection asString readStream)
>>>> The two differ.
>>>> The former stream position starts at editor startIndex - 1, while the
>>>> later stream position starts at 0.
>>>> The stream position is used by the Compiler/Parser for notifying the
>>>> SmalltalkEditor (or TextMorph) requestor.
>>>>
>>>> Now look at SmalltalkEditor>>notify: aString at: anInteger in: aStream
>>>>       "The compilation of text failed. The syntax error is noted as the argument,
>>>>       aString. Insert it in the text at starting character position anInteger."
>>>>
>>>>       | pos |
>>>>       pos := self selectionInterval notEmpty
>>>>               ifTrue: [self startIndex + anInteger - 1 ]
>>>>               ifFalse: [anInteger].
>>>>       self insertAndSelect: aString at: (pos max: 1)
>>>>
>>>> You see that it adds (startIndex - 1) to the error message insertion
>>>> position which is a Cuis change.
>>>> (The old ParagraphEditor did not).
>>>>
>>>> When #notify:at:in: is used for evaluating #selectionAsStream, all is
>>>> OK, the (startIndex - 1) is added only once.
>>>> But when it is used with #selectionForDoitAsStream, the offset is
>>>> counted twice (because already in the stream location passed in
>>>> anInteger parameter).
>>>>
>>>> I guess the hack #selectionForDoitAsStream that undo the Cuis clean-up
>>>> has been added because some other part of Parser/Encoder/Compiler is
>>>> messing with the requestor selection (like highlighting some part of
>>>> the source for user interaction in case of ambiguity).
>>>> Tthis is not a valid workaround.
>>>>
>>>> In Squeak, I did revert the Cuis clean-up just to keep some simple
>>>> compatibility with ParagraphEditor, but Pharo does not care about
>>>> that, so it should apply full simplification.
>>>> I suggest removing #selectionForDoitAsStream, and replace senders with
>>>> #selectionAsStream.
>>>> Then if there are some other feature broken we'll fix'em.
>>>> OK?
>>>
>>> Sure!!!
>>>
>>>>
>>>> Nicolas
>>>>
>>>
>>>