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@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@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@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 ]
>   !
>
>