The Inbox: Morphic-ct.1606.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: Morphic-ct.1606.mcz

commits-2
A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1606.mcz

==================== Summary ====================

Name: Morphic-ct.1606
Author: ct
Time: 7 December 2019, 4:53:17.106431 pm
UUID: 83ec1d59-dfa2-ca40-806d-809e76232100
Ancestors: Morphic-mt.1604

Restore original selection interval after any kind of do-it.

This is important if you print-it the following and skip the compiler hint:

| x |
x.
2

Before this commit, the answer was displayed just after x, not at the end of the snippet.

=============== Diff against Morphic-mt.1604 ===============

Item was changed:
  ----- Method: TextEditor>>evaluateSelectionAndDo: (in category 'do-its') -----
  evaluateSelectionAndDo: aBlock
  "Treat the current selection as an expression; evaluate it and invoke aBlock with the result."
+ | selectionInterval result rcvr ctxt |
- | result rcvr ctxt |
  self lineSelectAndEmptyCheck: [^ nil].
+ selectionInterval := self selectionInterval.
 
  (model respondsTo: #evaluateExpression:) ifTrue: [
  ^ aBlock value: (model perform: #evaluateExpression: with: self selection)].
 
  (model respondsTo: #doItReceiver)
  ifTrue: [ rcvr := model doItReceiver.
  ctxt := model doItContext]
  ifFalse: [rcvr := ctxt := nil].
  result := [
  rcvr class evaluatorClass new
  evaluate: self selectionAsStream
  in: ctxt
  to: rcvr
  environment: (model environment ifNil: [Smalltalk globals])
  notifying: self
  ifFail: [morph flash. ^ nil]
  logged: true.
  ]
  on: OutOfScopeNotification
  do: [ :ex | ex resume: true].
+ self selectInterval: selectionInterval.
+
-
  (model respondsTo: #expressionEvaluated:result:) ifTrue: [
  model perform: #expressionEvaluated:result: with: self selection with: result].
 
  ^aBlock value: result!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

marcel.taeumel
Hi Christoph,

-1

The print-it string has to be selected after the do-it so that one can easily delete it again. Otherwise, it would be quite cumbersome to do within-statement print-its. It know, that it got simpler because of "undo". Still...

Best,
Marcel

Am 07.12.2019 16:53:39 schrieb [hidden email] <[hidden email]>:

A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1606.mcz

==================== Summary ====================

Name: Morphic-ct.1606
Author: ct
Time: 7 December 2019, 4:53:17.106431 pm
UUID: 83ec1d59-dfa2-ca40-806d-809e76232100
Ancestors: Morphic-mt.1604

Restore original selection interval after any kind of do-it.

This is important if you print-it the following and skip the compiler hint:

| x |
x.
2

Before this commit, the answer was displayed just after x, not at the end of the snippet.

=============== Diff against Morphic-mt.1604 ===============

Item was changed:
----- Method: TextEditor>>evaluateSelectionAndDo: (in category 'do-its') -----
evaluateSelectionAndDo: aBlock
"Treat the current selection as an expression; evaluate it and invoke aBlock with the result."
+ | selectionInterval result rcvr ctxt |
- | result rcvr ctxt |
self lineSelectAndEmptyCheck: [^ nil].
+ selectionInterval := self selectionInterval.

(model respondsTo: #evaluateExpression:) ifTrue: [
^ aBlock value: (model perform: #evaluateExpression: with: self selection)].

(model respondsTo: #doItReceiver)
ifTrue: [ rcvr := model doItReceiver.
ctxt := model doItContext]
ifFalse: [rcvr := ctxt := nil].
result := [
rcvr class evaluatorClass new
evaluate: self selectionAsStream
in: ctxt
to: rcvr
environment: (model environment ifNil: [Smalltalk globals])
notifying: self
ifFail: [morph flash. ^ nil]
logged: true.
]
on: OutOfScopeNotification
do: [ :ex | ex resume: true].
+ self selectInterval: selectionInterval.
+
-
(model respondsTo: #expressionEvaluated:result:) ifTrue: [
model perform: #expressionEvaluated:result: with: self selection with: result].

^aBlock value: result!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

Christoph Thiede

Hi Marcel,


could you explain what you mean with "within-statement print-its"? This commit only ensures that after evaluating a section, the previous selection is restored. I could only think of the scenario that you turn off modal exclusivity of a dialog window and change something in the CodeHolder while the compiler is running. Do we want to support this scenario?


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Sonntag, 8. Dezember 2019 15:44:33
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Morphic-ct.1606.mcz
 
Hi Christoph,

-1

The print-it string has to be selected after the do-it so that one can easily delete it again. Otherwise, it would be quite cumbersome to do within-statement print-its. It know, that it got simpler because of "undo". Still...

Best,
Marcel

Am 07.12.2019 16:53:39 schrieb [hidden email] <[hidden email]>:

A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1606.mcz

==================== Summary ====================

Name: Morphic-ct.1606
Author: ct
Time: 7 December 2019, 4:53:17.106431 pm
UUID: 83ec1d59-dfa2-ca40-806d-809e76232100
Ancestors: Morphic-mt.1604

Restore original selection interval after any kind of do-it.

This is important if you print-it the following and skip the compiler hint:

| x |
x.
2

Before this commit, the answer was displayed just after x, not at the end of the snippet.

=============== Diff against Morphic-mt.1604 ===============

Item was changed:
----- Method: TextEditor>>evaluateSelectionAndDo: (in category 'do-its') -----
evaluateSelectionAndDo: aBlock
"Treat the current selection as an expression; evaluate it and invoke aBlock with the result."
+ | selectionInterval result rcvr ctxt |
- | result rcvr ctxt |
self lineSelectAndEmptyCheck: [^ nil].
+ selectionInterval := self selectionInterval.

(model respondsTo: #evaluateExpression:) ifTrue: [
^ aBlock value: (model perform: #evaluateExpression: with: self selection)].

(model respondsTo: #doItReceiver)
ifTrue: [ rcvr := model doItReceiver.
ctxt := model doItContext]
ifFalse: [rcvr := ctxt := nil].
result := [
rcvr class evaluatorClass new
evaluate: self selectionAsStream
in: ctxt
to: rcvr
environment: (model environment ifNil: [Smalltalk globals])
notifying: self
ifFail: [morph flash. ^ nil]
logged: true.
]
on: OutOfScopeNotification
do: [ :ex | ex resume: true].
+ self selectInterval: selectionInterval.
+
-
(model respondsTo: #expressionEvaluated:result:) ifTrue: [
model perform: #expressionEvaluated:result: with: self selection with: result].

^aBlock value: result!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

timrowledge
Christoph - you absolutely do *not* reselect the original text selection after a printit because we want the printed output selected. This is to allow quick deletion (since it is already selected) or copy etc.

There *is* a but shown up by the example you gave
>> | x |
>> x.
>> 2
where the compiler complaint about an unassigned var seems to mess up the selection but that is quite different. It looks like the selection gets messed up by the error handling. Somewhere code is setting the selection to be the apparently problematic variable name, which may well be sensible in some scenarios. However, if the result of the notification is to proceed the selection ought not have changed and the printIt result will be put in the right place.

It seems proper to change the selection if the user says to not proceed; that way the problem will be selected and I think that is plausibly helpful. If the user say to proceed then the selection should not be changed. The trick now is to find the place where it gets changed, and I have to admit that a quick scan for usages of OutOfScopeNotification didn't enlighten me much. Somewhere in the bowels of the compiler there will be a place.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: BBL: Branch on Burned out Light



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

Christoph Thiede

Hi tim,


you absolutely do *not* reselect the original text selection after a printit because we want the printed output selected. This is to allow quick deletion (since it is already selected) or copy etc.

I think I see what you mean, but did you try loading the commit? :) #printIt calls #evaluateSelectionAndDo:, and in the specified do block, it prints out the result and adjusts the selection to the output. So this will keep working fine. Or am I misunderstanding you?

Further, you are suggesting that we should only change the selection if the user cancels the compilation on a warning. (By the way, this selection is made in Parser >> #queryUndefined, for example. You can find out this quite easy by debugging the invocation of the dialog window and peeling back the stack.)
Personally, I would disagree with that change of behavior. Perhaps it may depend on your image's setting regarding focus & modal exclusivity, but I find it quite helpful to see the place where the error occurred selected while watching the message:


So my point is that the selection should be changed whenever a parser notification is raised.
Ergo, the selection might change at some point during parsing/compiling. If the user cancels the operation, this selection should stay so the user can fix the code directly. Otherwise, the warning is skipped and the temporary selection is no longer relevant to the user, so we should restore the previous selection.

What am I missing? :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von tim Rowledge <[hidden email]>
Gesendet: Sonntag, 8. Dezember 2019 20:12:15
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] The Inbox: Morphic-ct.1606.mcz
 
Christoph - you absolutely do *not* reselect the original text selection after a printit because we want the printed output selected. This is to allow quick deletion (since it is already selected) or copy etc.

There *is* a but shown up by the example you gave
>> | x |
>> x.
>> 2
where the compiler complaint about an unassigned var seems to mess up the selection but that is quite different. It looks like the selection gets messed up by the error handling. Somewhere code is setting the selection to be the apparently problematic variable name, which may well be sensible in some scenarios. However, if the result of the notification is to proceed the selection ought not have changed and the printIt result will be put in the right place.

It seems proper to change the selection if the user says to not proceed; that way the problem will be selected and I think that is plausibly helpful. If the user say to proceed then the selection should not be changed. The trick now is to find the place where it gets changed, and I have to admit that a quick scan for usages of OutOfScopeNotification didn't enlighten me much. Somewhere in the bowels of the compiler there will be a place.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: BBL: Branch on Burned out Light






pastedImage.png (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

timrowledge


> On 2019-12-08, at 1:11 PM, Thiede, Christoph <[hidden email]> wrote:
>
> I think I see what you mean, but did you try loading the commit? :)


No; I was about to leave for my flying club AGM, so didn't have time to do that. I was working from the descriptions.

> #printIt calls #evaluateSelectionAndDo:, and in the specified do block, it prints out the result and adjusts the selection to the output. So this will keep working fine. Or am I misunderstanding you?

No, that is what should happen in the normal case. The issue is what happens when there is an undefined (autocorrect wanted 'undefiled', which seems somehow very appropriate) variable in the text being evaluated. Me personally, I'd prefer a debugger in most cases, but I suppose this is an attempt to be helpful.

>
> Further, you are suggesting that we should only change the selection if the user cancels the compilation on a warning.

Not quite; the selection *at the end* should only be changed if the user chooses to stop. At that point it is quite reasonable to have errant variable name selected. And logically I'd have to agree that it is reasonable to do the selection as the notifier is opened. I think I'd prefer even more a secondary error-selection in a different colour but that would involve all sorts of other UI changes that might be problematic, so let's leave that aside.

> (By the way, this selection is made in Parser >> #queryUndefined, for example. You can find out this quite easy by debugging the invocation of the dialog window and peeling back the stack.)

Yah. The number of times I've done that over the last 4 decades probably needs a LargePositiveInteger to express.

> So my point is that the selection should be changed whenever a parser notification is raised.
> Ergo, the selection might change at some point during parsing/compiling. If the user cancels the operation, this selection should stay so the user can fix the code directly. Otherwise, the warning is skipped and the temporary selection is no longer relevant to the user, so we should restore the previous selection.
>
> What am I missing? :-)

A clean way to do that. Personally I'd prefer to see the selection fiddling removed from the #queryUndefined method and have that left to a handling method that knows it is working with a UI. So at a wild, late on a Sunday, guess, consider -
TextEditor>evaluateSelectionAndDo: aBlock
{fiddle with model respondsTo: crap that shouldn't be there either}
result := [wibbley-wobbley-timey-wimey-compile-and-do]
             on: OutOfScopeNotification
                 do:[ {stuff - but why just resume if something is out of scope?} ]
             on: UndefinedVariable
                 do: [:ex|
                   origSelection := self selectionInterval.
                   self selectInterval: ex selectionInterval "obviously the signal needs to carry this up from #queryUndefined"
                   answer := ex pass. "to get the answer from UI"
                   answer ifTrue:[self selectionInterval: origSelectin "may have logic backwards here"].
                   ex resume: answer]
{more weird respondsTo: faffing}

Why like this? It gathers together all the UI stuff in one place where the UI is a major player. It relieves the low-level compiler code of UI connections. It only changes the selection if it actually needs to (and remember that changing bits on glass is still relatively expensive). And it use on:do:on:do: - how often do we get to do layered exception handling?

{And the #respondsTo: stuff? Yuck. Those searches up the class chain scanning every method dictionary are expensive. #isKindOf: is massively cheaper and removing *those* was one of the things that I did to make Scratch many, many, times faster for the Pi. It would be so much nicer to avoid that construct }

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Any Sufficiently Advanced Incompetence Is Indistinguishable From Malice


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

marcel.taeumel
Hi Christoph,

I understand now. The fix is fine. However, both your description and picture are misleading. :-)

1. The description: " ... after evaluating a section, the previous selection is restored ..."

What you actually mean is after evaluating a selection and *before* printing the result.

2. The picture: 


Why is the "2" on an extra row? It looks like a print-out, which usually goes in the same line. Well, the "x" is undefined, which makes that "2" totally unrelated to the example.

***

Here is what it does:


Best,
Marcel

Am 09.12.2019 05:55:09 schrieb tim Rowledge <[hidden email]>:



> On 2019-12-08, at 1:11 PM, Thiede, Christoph wrote:
>
> I think I see what you mean, but did you try loading the commit? :)


No; I was about to leave for my flying club AGM, so didn't have time to do that. I was working from the descriptions.

> #printIt calls #evaluateSelectionAndDo:, and in the specified do block, it prints out the result and adjusts the selection to the output. So this will keep working fine. Or am I misunderstanding you?

No, that is what should happen in the normal case. The issue is what happens when there is an undefined (autocorrect wanted 'undefiled', which seems somehow very appropriate) variable in the text being evaluated. Me personally, I'd prefer a debugger in most cases, but I suppose this is an attempt to be helpful.

>
> Further, you are suggesting that we should only change the selection if the user cancels the compilation on a warning.

Not quite; the selection *at the end* should only be changed if the user chooses to stop. At that point it is quite reasonable to have errant variable name selected. And logically I'd have to agree that it is reasonable to do the selection as the notifier is opened. I think I'd prefer even more a secondary error-selection in a different colour but that would involve all sorts of other UI changes that might be problematic, so let's leave that aside.

> (By the way, this selection is made in Parser >> #queryUndefined, for example. You can find out this quite easy by debugging the invocation of the dialog window and peeling back the stack.)

Yah. The number of times I've done that over the last 4 decades probably needs a LargePositiveInteger to express.

> So my point is that the selection should be changed whenever a parser notification is raised.
> Ergo, the selection might change at some point during parsing/compiling. If the user cancels the operation, this selection should stay so the user can fix the code directly. Otherwise, the warning is skipped and the temporary selection is no longer relevant to the user, so we should restore the previous selection.
>
> What am I missing? :-)

A clean way to do that. Personally I'd prefer to see the selection fiddling removed from the #queryUndefined method and have that left to a handling method that knows it is working with a UI. So at a wild, late on a Sunday, guess, consider -
TextEditor>evaluateSelectionAndDo: aBlock
{fiddle with model respondsTo: crap that shouldn't be there either}
result := [wibbley-wobbley-timey-wimey-compile-and-do]
on: OutOfScopeNotification
do:[ {stuff - but why just resume if something is out of scope?} ]
on: UndefinedVariable
do: [:ex|
origSelection := self selectionInterval.
self selectInterval: ex selectionInterval "obviously the signal needs to carry this up from #queryUndefined"
answer := ex pass. "to get the answer from UI"
answer ifTrue:[self selectionInterval: origSelectin "may have logic backwards here"].
ex resume: answer]
{more weird respondsTo: faffing}

Why like this? It gathers together all the UI stuff in one place where the UI is a major player. It relieves the low-level compiler code of UI connections. It only changes the selection if it actually needs to (and remember that changing bits on glass is still relatively expensive). And it use on:do:on:do: - how often do we get to do layered exception handling?

{And the #respondsTo: stuff? Yuck. Those searches up the class chain scanning every method dictionary are expensive. #isKindOf: is massively cheaper and removing *those* was one of the things that I did to make Scratch many, many, times faster for the Pi. It would be so much nicer to avoid that construct }

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Any Sufficiently Advanced Incompetence Is Indistinguishable From Malice




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

marcel.taeumel
Here is another example of the bug that got addressed in this commit:



Best,
Marcel

Am 09.12.2019 10:02:25 schrieb Marcel Taeumel <[hidden email]>:

Hi Christoph,

I understand now. The fix is fine. However, both your description and picture are misleading. :-)

1. The description: " ... after evaluating a section, the previous selection is restored ..."

What you actually mean is after evaluating a selection and *before* printing the result.

2. The picture: 


Why is the "2" on an extra row? It looks like a print-out, which usually goes in the same line. Well, the "x" is undefined, which makes that "2" totally unrelated to the example.

***

Here is what it does:


Best,
Marcel

Am 09.12.2019 05:55:09 schrieb tim Rowledge <[hidden email]>:



> On 2019-12-08, at 1:11 PM, Thiede, Christoph wrote:
>
> I think I see what you mean, but did you try loading the commit? :)


No; I was about to leave for my flying club AGM, so didn't have time to do that. I was working from the descriptions.

> #printIt calls #evaluateSelectionAndDo:, and in the specified do block, it prints out the result and adjusts the selection to the output. So this will keep working fine. Or am I misunderstanding you?

No, that is what should happen in the normal case. The issue is what happens when there is an undefined (autocorrect wanted 'undefiled', which seems somehow very appropriate) variable in the text being evaluated. Me personally, I'd prefer a debugger in most cases, but I suppose this is an attempt to be helpful.

>
> Further, you are suggesting that we should only change the selection if the user cancels the compilation on a warning.

Not quite; the selection *at the end* should only be changed if the user chooses to stop. At that point it is quite reasonable to have errant variable name selected. And logically I'd have to agree that it is reasonable to do the selection as the notifier is opened. I think I'd prefer even more a secondary error-selection in a different colour but that would involve all sorts of other UI changes that might be problematic, so let's leave that aside.

> (By the way, this selection is made in Parser >> #queryUndefined, for example. You can find out this quite easy by debugging the invocation of the dialog window and peeling back the stack.)

Yah. The number of times I've done that over the last 4 decades probably needs a LargePositiveInteger to express.

> So my point is that the selection should be changed whenever a parser notification is raised.
> Ergo, the selection might change at some point during parsing/compiling. If the user cancels the operation, this selection should stay so the user can fix the code directly. Otherwise, the warning is skipped and the temporary selection is no longer relevant to the user, so we should restore the previous selection.
>
> What am I missing? :-)

A clean way to do that. Personally I'd prefer to see the selection fiddling removed from the #queryUndefined method and have that left to a handling method that knows it is working with a UI. So at a wild, late on a Sunday, guess, consider -
TextEditor>evaluateSelectionAndDo: aBlock
{fiddle with model respondsTo: crap that shouldn't be there either}
result := [wibbley-wobbley-timey-wimey-compile-and-do]
on: OutOfScopeNotification
do:[ {stuff - but why just resume if something is out of scope?} ]
on: UndefinedVariable
do: [:ex|
origSelection := self selectionInterval.
self selectInterval: ex selectionInterval "obviously the signal needs to carry this up from #queryUndefined"
answer := ex pass. "to get the answer from UI"
answer ifTrue:[self selectionInterval: origSelectin "may have logic backwards here"].
ex resume: answer]
{more weird respondsTo: faffing}

Why like this? It gathers together all the UI stuff in one place where the UI is a major player. It relieves the low-level compiler code of UI connections. It only changes the selection if it actually needs to (and remember that changing bits on glass is still relatively expensive). And it use on:do:on:do: - how often do we get to do layered exception handling?

{And the #respondsTo: stuff? Yuck. Those searches up the class chain scanning every method dictionary are expensive. #isKindOf: is massively cheaper and removing *those* was one of the things that I did to make Scratch many, many, times faster for the Pi. It would be so much nicer to avoid that construct }

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Any Sufficiently Advanced Incompetence Is Indistinguishable From Malice




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-ct.1606.mcz

Christoph Thiede

Hi Marcel,


yes, thank you for the clarification! It is very helpful for me to hear the weaknesses of my explanation. I will try to be more precise the next time :-)


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Montag, 9. Dezember 2019 10:06:33
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Morphic-ct.1606.mcz
 
Here is another example of the bug that got addressed in this commit:



Best,
Marcel

Am 09.12.2019 10:02:25 schrieb Marcel Taeumel <[hidden email]>:

Hi Christoph,

I understand now. The fix is fine. However, both your description and picture are misleading. :-)

1. The description: " ... after evaluating a section, the previous selection is restored ..."

What you actually mean is after evaluating a selection and *before* printing the result.

2. The picture: 


Why is the "2" on an extra row? It looks like a print-out, which usually goes in the same line. Well, the "x" is undefined, which makes that "2" totally unrelated to the example.

***

Here is what it does:


Best,
Marcel

Am 09.12.2019 05:55:09 schrieb tim Rowledge <[hidden email]>:



> On 2019-12-08, at 1:11 PM, Thiede, Christoph wrote:
>
> I think I see what you mean, but did you try loading the commit? :)


No; I was about to leave for my flying club AGM, so didn't have time to do that. I was working from the descriptions.

> #printIt calls #evaluateSelectionAndDo:, and in the specified do block, it prints out the result and adjusts the selection to the output. So this will keep working fine. Or am I misunderstanding you?

No, that is what should happen in the normal case. The issue is what happens when there is an undefined (autocorrect wanted 'undefiled', which seems somehow very appropriate) variable in the text being evaluated. Me personally, I'd prefer a debugger in most cases, but I suppose this is an attempt to be helpful.

>
> Further, you are suggesting that we should only change the selection if the user cancels the compilation on a warning.

Not quite; the selection *at the end* should only be changed if the user chooses to stop. At that point it is quite reasonable to have errant variable name selected. And logically I'd have to agree that it is reasonable to do the selection as the notifier is opened. I think I'd prefer even more a secondary error-selection in a different colour but that would involve all sorts of other UI changes that might be problematic, so let's leave that aside.

> (By the way, this selection is made in Parser >> #queryUndefined, for example. You can find out this quite easy by debugging the invocation of the dialog window and peeling back the stack.)

Yah. The number of times I've done that over the last 4 decades probably needs a LargePositiveInteger to express.

> So my point is that the selection should be changed whenever a parser notification is raised.
> Ergo, the selection might change at some point during parsing/compiling. If the user cancels the operation, this selection should stay so the user can fix the code directly. Otherwise, the warning is skipped and the temporary selection is no longer relevant to the user, so we should restore the previous selection.
>
> What am I missing? :-)

A clean way to do that. Personally I'd prefer to see the selection fiddling removed from the #queryUndefined method and have that left to a handling method that knows it is working with a UI. So at a wild, late on a Sunday, guess, consider -
TextEditor>evaluateSelectionAndDo: aBlock
{fiddle with model respondsTo: crap that shouldn't be there either}
result := [wibbley-wobbley-timey-wimey-compile-and-do]
on: OutOfScopeNotification
do:[ {stuff - but why just resume if something is out of scope?} ]
on: UndefinedVariable
do: [:ex|
origSelection := self selectionInterval.
self selectInterval: ex selectionInterval "obviously the signal needs to carry this up from #queryUndefined"
answer := ex pass. "to get the answer from UI"
answer ifTrue:[self selectionInterval: origSelectin "may have logic backwards here"].
ex resume: answer]
{more weird respondsTo: faffing}

Why like this? It gathers together all the UI stuff in one place where the UI is a major player. It relieves the low-level compiler code of UI connections. It only changes the selection if it actually needs to (and remember that changing bits on glass is still relatively expensive). And it use on:do:on:do: - how often do we get to do layered exception handling?

{And the #respondsTo: stuff? Yuck. Those searches up the class chain scanning every method dictionary are expensive. #isKindOf: is massively cheaper and removing *those* was one of the things that I did to make Scratch many, many, times faster for the Pi. It would be so much nicer to avoid that construct }

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Any Sufficiently Advanced Incompetence Is Indistinguishable From Malice