[squeak-dev] The Inbox: Morphic-ct.1606.mcz

Thiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.de
Mon Dec 9 10:44:53 UTC 2019


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 <squeak-dev-bounces at lists.squeakfoundation.org> 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:

[cid:bcf0e08c-705b-434b-a8f6-53c2ef0a159b]

Best,
Marcel

Am 09.12.2019 10:02:25 schrieb Marcel Taeumel <marcel.taeumel at hpi.de>:

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:

[cid:b9b0542e-d402-448c-ab24-691f9d144c58]

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:
[cid:280fc226-dc0d-4a32-91d7-a723139b55d7]

Best,
Marcel

Am 09.12.2019 05:55:09 schrieb tim Rowledge <tim at rowledge.org>:


> 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; tim at rowledge.org; http://www.rowledge.org/tim
Any Sufficiently Advanced Incompetence Is Indistinguishable From Malice


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191209/54285117/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 13715 bytes
Desc: image.png
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191209/54285117/attachment.png>


More information about the Squeak-dev mailing list