Hi all, Hi Marcel,


<http://www.hpi.de/>
> We should find a way to get rid of #headingAndAutoselectForLiteral:do:. That very name indicates that there is something wrong with the overall invocation of SystemNavigation's public protocol.

Hm, I guess the simplest way would be splitting up #headingAndAutoselectForLiteral:do: into #headingForLiteral: and #stringForLiteral: or something like this. Looking at the latter, this should be equivalent to [literal name], which is returns a valid string each for Symbol, String, LookupKey, Integer, and Boolean, so we would only need to implemented the former. What do you think? :-)

Best,
Christoph
________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Donnerstag, 3. September 2020 15:15:31
An: squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Tools-ct.986.mcz

Hi --

#headingAndAutoselectForLiteral:do: is a private helper function to disguise a rather constant value, which is a prefix for a tool's label. It should not be used outside SystemNavigation. That would make things even worse.

The basic issue here is that there is no specific object for the concerns "user" or "sender". So, it is tediously derived or forwarded in a functional style.

We should find a way to get rid of #headingAndAutoselectForLiteral:do:. That very name indicates that there is something wrong with the overall invocation of SystemNavigation's public protocol.

Best,
Marcel


Am 03.09.2020 15:02:06 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:

Hi Chris,


thanks for your feedback!

> For your code contributions in general, please allow methods to have only a single exit as much as possible, as in the attached.

I think this is very much a question of favor - personally, I prefer guard clauses over the functional style unless both code paths have the same relevance for the whole method. In my opinion, an empty message list is clearly a secondary edge case only. :-)

> I like the multilingual change, but the purpose of using #headingAndAutoselectForLiteral:do: here wasn't obvious to me.

Yes, this is not really an intuitive message name ... What do you think about calling it #readLiteral:withHeadingAndStringDo: instead (plus making it public)?

Best,
Christoph
________________________________
Von: Chris Muller <asqueaker at gmail.com>
Gesendet: Mittwoch, 2. September 2020 23:59:46
An: squeak dev; Thiede, Christoph
Betreff: Re: [squeak-dev] The Inbox: Tools-ct.986.mcz

Hi Christoph,

For your code contributions in general, please allow methods to have
only a single exit as much as possible, as in the attached.

I like the multilingual change, but the purpose of using
#headingAndAutoselectForLiteral:do: here wasn't obvious to me.

Best,
  Chris

On Wed, Sep 2, 2020 at 9:56 AM <commits at source.squeak.org> wrote:
>
> Christoph Thiede uploaded a new version of Tools to project The Inbox:
> http://source.squeak.org/inbox/Tools-ct.986.mcz
>
> ==================== Summary ====================
>
> Name: Tools-ct.986
> Author: ct
> Time: 2 September 2020, 4:55:38.538083 pm
> UUID: b4cdf611-f04b-0b40-8f61-34429f414cca
> Ancestors: Tools-ct.985
>
> Fixes MNU when adding senders of a non-string literal to a message trace (at the end, FindText was set to a number or something similar). Improves multilingual support.
>
> =============== Diff against Tools-ct.985 ===============
>
> Item was changed:
>   ----- Method: MessageTrace>>addParentMethodsSending: (in category 'building') -----
>   addParentMethodsSending: selectorSymbol
>
> +       ^ self systemNavigation
> +               headingAndAutoselectForLiteral: selectorSymbol
> +               do: [:label :autoSelect |
> +                       | methodsList |
> +                       methodsList := self systemNavigation allCallsOn: selectorSymbol.
> +                       methodsList ifEmpty: [
> +                               ^ self inform: ('There are no {1}' translated format: {label})].
> +                       self
> -       | methodsList |
> -       (methodsList := self systemNavigation allCallsOn: selectorSymbol) isEmpty
> -               ifTrue:
> -                       [ ^(PopUpMenu labels: ' OK ')
> -                               startUpWithCaption: 'There are no methods that send ', selectorSymbol ]
> -               ifFalse:
> -                       [ self
>                                 addParentMessages: methodsList
> +                               autoSelectString: autoSelect]
> -                               autoSelectString: selectorSymbol ]
>   !
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200908/4920371e/attachment-0001.html>