Improving the SystemNavigation browseMessageList... stuff

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

Improving the SystemNavigation browseMessageList... stuff

timrowledge
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.................



Reply | Threaded
Open this post in threaded view
|

Re: Improving the SystemNavigation browseMessageList... stuff

timrowledge


> 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



Reply | Threaded
Open this post in threaded view
|

Re: Improving the SystemNavigation browseMessageList... stuff

Jakob Reschke
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

Reply | Threaded
Open this post in threaded view
|

Re: Improving the SystemNavigation browseMessageList... stuff

Jakob Reschke
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

Reply | Threaded
Open this post in threaded view
|

Re: Improving the SystemNavigation browseMessageList... stuff

timrowledge
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.



Reply | Threaded
Open this post in threaded view
|

Re: Improving the SystemNavigation browseMessageList... stuff

timrowledge
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.