On problem with the current solution (from 12144) is that TextModels
readSelectionHolder is not updated when setSelection: is called.
So, if you read the selection from the PluggableTextMorph it has
always the same value (from its initialization) and not the actual selection
(if you use the mouse to set a new selection).

Compare TextModel>>setSelection: in Pharo3 and Pharo2 ( keep in mind that
Pharo3 uses a MorphicTextAdapter between the model (TextModel) and the view
(PluggableTextMorph).

But I don't like this version (12144 with correct readSelectionHolder
updating).
I find it pretty confusing how textselection is handled in TextModel,
MorphicTextAdapter, PluggableTextMorph
and TextEditor.

And look at issue 12487 <https://pharo.fogbugz.com/default.asp?12487> and
my last comment in 12389
<https://pharo.fogbugz.com/default.asp?12389>(this is about
textselection initializating from
building the TextMorph with spec, TextModels spec does not handle the
current selection.
And I couldn't find a way to tell SpecInterpreter how to use the actual
selection (if it is of kind Interval))


Nicolai








2013/12/27 Andrei Chis <[email protected]>

> PluggableTextMorph maintains its own selectionInterval.
>
> When the PluggableTextMorph >>selectionInterval: it is called it sets the
> selection in the model.
> This will trigger MorphicTextAdapter>>setSelectionFromModel: that should
> adapt the selection in the morph:
> MorphicTextAdapter >>setSelectionFromModel: aSelection
> self widget ifNotNil: [:w |
> w selectionInterval ~= aSelection ifTrue: [
>  w setSelection: aSelection ]].
>
> However, the second test (w selectionInterval ~= aSelection) will always
> return false. This is because
> PluggableTextMorph>>selectionInterval has the following implementation
>  '^ textMorph editor selectionInterval'
>
> And in the editor the selection is already correct (even
> before  PluggableTextMorph >>selectionInterval: is called).
>
> Actually most of the methods from PluggableTextMorph that change the
> selection also explicitly update the
> selectionInterval instance variable.
>
> So the previous solution should work.
>
> @Doru this is related with Spec, so we should not have it in Glamour.
>
>
>
> On Fri, Dec 27, 2013 at 5:07 PM, Benjamin <
> [email protected]> wrote:
>
>> I think that when the model is notified, it will set it in the morph
>> So it will happen twice
>>
>> Ben
>>
>> On 27 Dec 2013, at 15:52, Andrei Chis <[email protected]> wrote:
>>
>> Wouldn't then explicitly setting the selection of the morph before
>> setting it in the model be ok?
>>
>> PluggableTextMorph>>selectionInterval: sel
>>         selectionInterval := sel.
>> setSelectionSelector
>>  ifNotNil: [ self model perform:setSelectionSelector with: sel ]
>>
>>
>> On Fri, Dec 27, 2013 at 4:28 PM, Benjamin <
>> [email protected]> wrote:
>>
>>> ok :(
>>> Then I run out of brilliant idea :P
>>>
>>>
>>> Ben
>>>
>>> On 27 Dec 2013, at 15:25, Andrei Chis <[email protected]>
>>> wrote:
>>>
>>> Adding 'self changed' at the end of the current 
>>> PluggableTextMorph>>selectionInterval:
>>> doesn't solve the problem.
>>>
>>>
>>> On Fri, Dec 27, 2013 at 4:15 PM, Benjamin <
>>> [email protected]> wrote:
>>>
>>>> Because when the focus changes, the morph is proposed to redraw itself
>>>> Try with a
>>>>
>>>>     self changed
>>>>
>>>> at the end
>>>>
>>>> Ben
>>>>
>>>> On 27 Dec 2013, at 14:44, Andrei Chis <[email protected]>
>>>> wrote:
>>>>
>>>>
>>>> The change that seems to have caused this problem is in
>>>> PluggableTextMorph>>selectionInterval: The selectionInterval instance
>>>> variable of the morph is not set any more; only the model is updated, which
>>>> doesn't seem to update the morph.
>>>>
>>>> PluggableTextMorph>>selectionInterval: sel
>>>> setSelectionSelector
>>>> ifNil: [ selectionInterval := sel ]
>>>>  ifNotNil: [ self model perform:setSelectionSelector with: sel ]
>>>>
>>>> This could be solved like bellow, though I'm not sure it is the best
>>>> solution.
>>>> If I put a halt in PluggableTextMorph >>inspectIt and hit proceed the
>>>> the selection is inspected correctly, without this modification.
>>>>
>>>> PluggableTextMorph>>selectionInterval: sel
>>>>         selectionInterval := sel.
>>>> setSelectionSelector
>>>> ifNotNil: [ self model perform:setSelectionSelector with: sel ]
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Fri, Dec 27, 2013 at 3:11 PM, Marcus Denker 
>>>> <[email protected]>wrote:
>>>>
>>>>>
>>>>> On 27 Dec 2013, at 13:01, Stéphane Ducasse <[email protected]>
>>>>> wrote:
>>>>>
>>>>> > Hi guys
>>>>> >
>>>>> > we should not let the system in such state. We cannot inspect print
>>>>> expression in the debugger.
>>>>> > Does anybody have an idea of the change that broke it?
>>>>> >
>>>>> Yes:
>>>>>
>>>>>         https://pharo.fogbugz.com/f/cases/12144/TextModel-getSelection
>>>>>
>>>>> it was added in 655, so for reverting the easiest is to dl 654, do a
>>>>> merge and look at all the changes
>>>>> (there are just 4-5 that are not re-catorizations).
>>>>>
>>>>> This is on my TODO next…
>>>>>
>>>>>         Marcus
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>

Reply via email to