Smalltalk › Squeak › Squeak - Dev

Improving the SystemNavigation browseMessageList... stuff

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

9 messages Options

timrowledge

Improving the SystemNavigation browseMessageList... stuff

I just had reason to take a look at the list of unimplemented messages in my working image. I was a bit surprised to see such a large number *not* related to my current work - in fact the latest trunk image claims *577* cases. Obviously a few are legitimate places where some test needs an unimplemented symbol to find but still, that's a lot of places where sometihng is going to go wrong.

It has to be said that VeryPickyMorph>>#justDroppedInto:event: is especially amusing in this regard. Maybe not so picky after all?

What struck me though was how unhelpful the browser ends up being in this case. What we get is a long list of methods where
a) there are duplicates - see MenuMorph>>#setTitleParametersFor: for an example. It apparently sends titleColor, titleBorderStyle, titleBorderColor and titleBorderWidth and appears 4 time in the browser. I'd claim that it would be far better to build some variety of Set of MethodReferences in MessageSet>>#initializeMessageList:

b) we lose the useful info about which message is unimplemented in the parsing in MessageSet>>#initializeMessageList:. Oddly, I'd swear I've seen something, somewhere, sometime, that could holds onto some more info to pass onto a browser. Anybody? And of course, the browser would need some extensions to actually display the unimplemented message. This might even be a place where Shout would actually appeal even to me.

The first question really has to be whether anyone has already tackled this stuff?

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
A computer's attention span is only as long as its extension cor.................

timrowledge

Re: Improving the SystemNavigation browseMessageList... stuff

> On 2020-10-17, at 1:37 PM, tim Rowledge <[hidden email]> wrote:
>
> What struck me though was how unhelpful the browser ends up being in this case. What we get is a long list of methods where
> a) there are duplicates - see MenuMorph>>#setTitleParametersFor: for an example. It apparently sends titleColor, titleBorderStyle, titleBorderColor and titleBorderWidth and appears 4 time in the browser. I'd claim that it would be far better to build some variety of Set of MethodReferences in MessageSet>>#initializeMessageList:

Making a Set instead is easy enough, and then if we convert that to a SortedCollection we get a more useful result. It only takes trivial changes to two methods to do this.

Unfortunately it also reveals that the SystemNavigation>>#browseMessageList:name:autoSelect: is in need of some improvement since the 'nothing selected' label is set to the proffered label + the raw incoming collection size - and that is not correct after the Set conversion.

This raises a philosophical question about how we really want MessageSet browsers to behave. If we create a list with one method reference repeated 100 times, is there any reason at all anyone would want to see a 100 entry browser list? And is there any reason anyone can think of to not move that label-futzing code into the actual tool instead of a rather unwarranted assumption in the SystemNavigation code?

There a promising (if complicated) method #adjustWindowTitleAfterFiltering but unfortunately there is no containing window at the relevant time and so it cannot do the job.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: IBLU: Ignore Basic Laws of Universe

Jakob Reschke

Re: Improving the SystemNavigation browseMessageList... stuff

Without the unimplemented selector being highlighted, I cannot see a reason
why to display a method twice. *But* with Environments you can have the same
combination of class name and selector twice or more in the image and these
should still appear separate. That means the deduplication must not rely
just on the string representation of the method. Instead, MethodReference
must be used.

--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Jakob Reschke

Re: Improving the SystemNavigation browseMessageList... stuff

In reply to this post by timrowledge

timrowledge wrote
> b) we lose the useful info about which message is unimplemented in the
> parsing in MessageSet>>#initializeMessageList:. Oddly, I'd swear I've seen
> something, somewhere, sometime, that could holds onto some more info to
> pass onto a browser. Anybody?

Do you mean something like "Senders of" selecting the sent selector in a
found method?
In MessageTrace that revolves around the autoSelectStrings variable. In
MessageSet it is just one autoSelectString.

--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

timrowledge

Re: Improving the SystemNavigation browseMessageList... stuff

In reply to this post by Jakob Reschke

> On 2020-10-18, at 4:46 AM, Jakob Reschke <[hidden email]> wrote:
>
> Without the unimplemented selector being highlighted, I cannot see a reason
> why to display a method twice. *But* with Environments you can have the same
> combination of class name and selector twice or more in the image and these
> should still appear separate. That means the deduplication must not rely
> just on the string representation of the method. Instead, MethodReference
> must be used.

MethodReference is indeed used here. See MessageSet>>initializeMessageList: for example. It appears to have properly environment aware #= but just possibly someone might want to change the #<=. I'm trying to avoid worrying about environments, so I won't be that person.

The bit needed to make this tool really useful is the ability to highlight the offending unimplemented message(s) in the selected method. MessageTrace adds a list of individually settable autoSelectStrings but of course the entire edit model expects a single contiguous selection and so we can't easily (mis)use that if there are several unimplemented messages in one method. Really, using a text selection as a highlight is pretty much useless as soon as you have a multipart keyword message.

Maybe this is something Shout could actually do for us? And in the process perhaps we could end up highlighting the actual messages and not the same characters in a comment just because it is the first place in some text that we find those letters? If we could inform Shout that we want a special highlighting for the attached set of message selectors then *all* the browsers where we want some "show this found thing" would be improvable.

We also need a good way to pass the miscreant symbols along from the original input - which is in the form of an OrderedCollection of Strings like
'MenuMorph addTitle:icon:updatingSelector:updateTarget: calls: titleFont'
and get the 'titleFont' to some object that the browser will use to trigger the highlighting. I suppose a subclass of MethodReference might solve that, or a simple composite of a MethodReference and a Set of symbols.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Useful random insult:- Solid concrete from the eyebrows backwards.

timrowledge

Re: Improving the SystemNavigation browseMessageList... stuff

OK, review time !

See inbox Tools-tpr.1003 and System-tpr.1181 for some fairly simple changes to how the message list is built (as a set and sorted etc) and improvements to how the window label is derived.

The only visible change should be that when the window initially opens the label will reflect the fact that the first item is actually selected - the prior code actually started in a weird state with the label that should be used when nothing is selected. I think it is more correct now but one might argue that nothing should be selected by default for this sort of browser.

Now, about that improvement to the Shout stuff...

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Useful random insult:- Wasn't fully debugged before being released.

Christoph Thiede

Re: Improving the SystemNavigation browseMessageList... stuff

Hi Tim,

some scattered comments on your changes: It's a great idea to refactor this windowTitle stuff!

Personally, I think it would look nicer if you could leave out the spaces between the brackets, e.g. "Implementors of initialize [987]" instead of "Implementors of initialize [ 987 ]".

Not a regression, but still a bug: Select 42 in some method in a MessageTrace and browse senders -> Error: Instances of SmallInteger are not indexable from #findAgainNow. Is this related to your changes?

> Possibly better to do asOrderedCollection sort ?

Sounds reasonable.

> Now, about that improvement to the Shout stuff...

Your ideas sound interesting. We have collected so many ideas on what Shout/attributes could do else (ShoutAttribute, highlight searched text, and here are some other rough ideas: highlight all occurrences of the currently selected word/selector as you may know it from VS Code; automatically style hyperlinks with a TextURL (ask Tom Beckmann for more details on this idea) or class names with a TextLink; a text attribute that adds a tooltip to a certain subtext; allow to browse a word by Ctrl + clicking it, probably again via a text attribute; ...). Maybe we should finally split up the TextAttribute hierarchy into two layers, one low-level layer for emphasis, indent, etc., and one high-level layer for attributes that need to be converted into low-level attributes before display, e.g. StyleAttribute, HighlightAttribute, etc. This conversion could be done by a shout-like mechanism. Yeah, these are indeed rough ideas, just wanted to collect them anywhere. :-)

Best,

Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von tim Rowledge <[hidden email]>
Gesendet: Montag, 19. Oktober 2020 02:32:39
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Improving the SystemNavigation browseMessageList... stuff

pastedImage.png (10K) Download Attachment

Carpe Squeak!

timrowledge

Re: Improving the SystemNavigation browseMessageList... stuff

> On 2020-10-24, at 12:13 PM, Thiede, Christoph <[hidden email]> wrote:
>
> Hi Tim,
>
> some scattered comments on your changes: It's a great idea to refactor this windowTitle stuff!
>
> Personally, I think it would look nicer if you could leave out the spaces between the brackets, e.g. "Implementors of initialize [987]" instead of "Implementors of initialize [ 987 ]".

No argument there.

>
> Not a regression, but still a bug: Select 42 in some method in a MessageTrace and browse senders -> Error: Instances of SmallInteger are not indexable from #findAgainNow. Is this related to your changes?

No, but certainly a bug. It's related to - I suspect - the problem I noticed in MessageTrace>>#getImplementorNamed: where we find that occasionally the parameter 'selectorSymbol' is actually a Character. Which is indeed not indexable.
*That* seems to stem from TextEditor>>implementorsOfIt where we see that we are using
``` self selectedLiteral ifNotNil:
[:aLiteral| ^self model browseAllImplementorsOf: aLiteral].
```
( a change tagged by Marcel about 6 months ago) and in this case aLiteral is the Character [

It's possible the code finding the literal is not working right. It's possible the concept is faulty - should it even try to find all the 'implementors' of something like $[ ?

One of my changes 'fixes' at least part of the problem but I couldn't replicate your exact case.

>
> > Possibly better to do asOrderedCollection sort ?
>
> Sounds reasonable.

Yeah, it works and reads better.

>
> > Now, about that improvement to the Shout stuff...
>
> Your ideas sound interesting. We have collected so many ideas on what Shout/attributes could do else (ShoutAttribute, highlight searched text, and here are some other rough ideas:

> highlight all occurrences of the currently selected word/selector as you may know it from VS Code;

No. Absolutely no. Nope. Nopetty nope with nope-sauce on top. I've encountered this in the horrific code editor used by WordPress and it is *horrible*. It's also an poorly bounded expense of time. I mean seriously, if you are editing a megabyte file and select 'e' wold it realyl make any sense at all to search out and highlight every damn 'e' ? Madness. Typical 'UI cleverness' from microsoft, the people that wouldn't know a good UI if it bit them on the cerebral cortex.

> automatically style hyperlinks with a TextURL (ask Tom Beckmann for more details on this idea) or class names with a TextLink; a text attribute that adds a tooltip to a certain subtext; allow to browse a word by Ctrl + clicking it, probably again via a text attribute; ...).

This is essentially the data parsing and marking stuff that is used in (amongst others) Apple Mail and can be used to do 'useful things' like spotting a likely appointment to add to your calendar. Again, there are potential performance issues that need careful consideration and there would need to be ways to specify which things get done for each situation - I really don't want time spent on searching for appointments in source code! Also, heavily covered by patents that have been litigated and cost people serious money - I know because I worked on one or two cases.

Given the lack of people screaming to not do it, I'll move a slightly updated version into the trunk and we'll see how it goes.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
A bad random number generator: 1, 1, 1, 1, 1, 4.33e+67, 1, 1, 1

Christoph Thiede

Re: Improving the SystemNavigation browseMessageList... stuff

> Not a regression, but still a bug: Select 42 in some method in a MessageTrace and browse senders -> Error: Instances of SmallInteger are not indexable from #findAgainNow. Is this related to your changes?

Ah, I believe I already fixed this one in Tools-ct.986. Sorry, I'm losing the overview of my own changes ...

However, this will be a merge conflict with your changes. Sigh.

> > highlight all occurrences of the currently selected word/selector as you may know it from VS Code;

> No.

Alright, a matter of taste. No problem ... May I say the bad word once again? We could make it a *preference* :D

It can be useful at some times and useless at other times. In Squeak, methods are usually short so the possible outcome is smaller, but still it could help me to identify recurring literals at a glance, just for example.

I was just brainstorming ... There are even more things that currently are too much coupled to the editor imho ... For instance, support for displaying/highlighting opposite brackets or everything between two brackets could be improved as well ...

> Given the lack of people screaming to not do it, I'll move a slightly updated version into the trunk and we'll see how it goes.

Great, thank you!

Best,

Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von tim Rowledge <[hidden email]>
Gesendet: Montag, 26. Oktober 2020 00:52:13
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Improving the SystemNavigation browseMessageList... stuff

Carpe Squeak!