The Inbox: Tools-ct.986.mcz

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

The Inbox: Tools-ct.986.mcz

commits-2
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 ]
  !


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Christoph Thiede

Can we make SystemNavigation >> #headingAndAutoselectForLiteral:do: non-private?


Von: Squeak-dev <[hidden email]> im Auftrag von [hidden email] <[hidden email]>
Gesendet: Mittwoch, 2. September 2020 16:55:49
An: [hidden email]
Betreff: [squeak-dev] The Inbox: Tools-ct.986.mcz
 
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 ]
  !




Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Chris Muller-3
In reply to this post by commits-2
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 <[hidden email]> 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 ]
>   !
>
>



MessageTrace-addParentMethodsSending.st (824 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Christoph Thiede

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

marcel.taeumel
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 <[hidden email]>:

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Christoph Thiede

Hi all, Hi Marcel,


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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Eliot Miranda-2
In reply to this post by marcel.taeumel
Hi Marcel,


On Sep 3, 2020, at 6:15 AM, Marcel Taeumel <[hidden email]> wrote:


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.

+1.  Making a SystemNavigation stateful so that it can be assigned something for auto select would be great.  It could also take a pattern not just a string, and hence deal with multi keyword messages better.

If we separate the search and presentation functions there are several benefits too:

- we can get a calculus for search, so that we can have and and or. So something like search for all calls on methods that refer to selector a and selector b in class hierarchy c are easily expressed as the and of three elemental searches, fir selector a, for selector b, for class hierarchy c

- we can get rid of the duplication in the allFoo and browseAllFoo protocols, keeping the search and just sending browse to the result.

- we can derive results in the forms we’d like, so a “navigation” could answer methods, classes, packages, method references, etc

As a suggested implementation Object>>smalltalk or Object>>navigation answers a new SystemSearch instance; SystemSearch instances understand basic queries (allCallsOn:, allImplementorsOf: etc), and an and: and or: protocol that take either another SystemSearch or a message, and answer or update the receiver.  Then at any time one can send autoSelect: to the SystemSearch, and browse to see its results.

BTW, a concise way of creating a message might be cool; Message selector: #allCallsOn: argument: 123 is fine but a little verbose and tedious to type.  #allCallsOn: << { 123 } might be nice, or a little too cryptic.  I don’t know.

Btw, Andreas hated and I hate typing systemNavigation; so long.  I implement sn in Object as an abbreviation.  Hence my suggestion of smalltalk or navigator above.  search would be even better:

     (self search
          allCallsOn: #systemNavigation) “sets systemNavigation as the default auto select string”
          and: #inPackage: << { #Tools };
          browse

is much more flexible than 
    self sysyemNavigation browseAllCallsOn: #systemNavigation loxalToPackage: #Tools

Or have and: be the default combinatorial so one can cascade ands:

     self search
          allCallsOn: #systemNavigation;
          inPackage: #Tools;
          browse

I like this last one.


Best,
Marcel

Am 03.09.2020 15:02:06 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Chris Muller-3


On Tue, Sep 29, 2020 at 7:18 AM Eliot Miranda <[hidden email]> wrote:
Hi Marcel,


On Sep 3, 2020, at 6:15 AM, Marcel Taeumel <[hidden email]> wrote:


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.

+1.  Making a SystemNavigation stateful so that it can be assigned something for auto select would be great.  It could also take a pattern not just a string, and hence deal with multi keyword messages better.

If we separate the search and presentation functions there are several benefits too:

- we can get a calculus for search, so that we can have and and or. So something like search for all calls on methods that refer to selector a and selector b in class hierarchy c are easily expressed as the and of three elemental searches, fir selector a, for selector b, for class hierarchy c

- we can get rid of the duplication in the allFoo and browseAllFoo protocols, keeping the search and just sending browse to the result.

- we can derive results in the forms we’d like, so a “navigation” could answer methods, classes, packages, method references, etc

As a suggested implementation Object>>smalltalk or Object>>navigation answers a new SystemSearch instance; SystemSearch instances understand basic queries (allCallsOn:, allImplementorsOf: etc), and an and: and or: protocol that take either another SystemSearch or a message, and answer or update the receiver.  Then at any time one can send autoSelect: to the SystemSearch, and browse to see its results.

BTW, a concise way of creating a message might be cool; Message selector: #allCallsOn: argument: 123 is fine but a little verbose and tedious to type.  #allCallsOn: << { 123 } might be nice, or a little too cryptic.  I don’t know.

+1, I kinda like that!   Although, some objects already implement #<< , hmm...

For years, I've actually been wanting a convenience constructor for MethodReference.  I _wish_ I could write

     Integer>>#gcd:

and get a MethodReference instead of a CompiledMethod, since I can easily send #compiledMethod to a MethodReference if I want that (which, I never do, I only ever want the Reference).  I suppose I could implement CompiledMethod>>#asMethodReference, it'd just be so much nicer if it were the other way...
 
Btw, Andreas hated and I hate typing systemNavigation; so long.  I implement sn in Object as an abbreviation.  Hence my suggestion of smalltalk or navigator above.  search would be even better:

     (self search
          allCallsOn: #systemNavigation) “sets systemNavigation as the default auto select string”
          and: #inPackage: << { #Tools };
          browse

is much more flexible than 
    self sysyemNavigation browseAllCallsOn: #systemNavigation loxalToPackage: #Tools

Or have and: be the default combinatorial so one can cascade ands:

     self search
          allCallsOn: #systemNavigation;
          inPackage: #Tools;
          browse

I like this last one.

A great improvement!  Except #search.  We should stay with #systemNavigation.  We developers have autoComplete, etc., we shouldn't poach Object>>#search from the API namespace from users.

For the ultimate method-selecting power in the universe, MaBehaviorFinder takes the above concept one step further.  Instead of always starting with "all" and filtering, it provides both adding AND removing filters.  So the above example could be:

       self systemNavigation
            addCallsOn: #systemNavigation ;         "adding filter"
            selectPackage: #Tools ;                         "removing filter"
            browse

Or, equally:

       self systemNavigation
            addPackage: #Tools ;                          "adding filter"
            selectCallsOn: #systemNavigation      "removing filter"
            browse

So, various "add" API, ADDS methods to the result, while various "select" API, removes them from the result.  (There's even a third family, "reject" which select the opposite of "select").

There's quite a few methods, but executing them in succession on a stateful instance, as you've described for SystemNavigation here, is exactly how it works, and it's fabulous!   :-D

Just in case anyone is interested in taking a peek at it for inspiration, the following will pop it up:

      Installer new merge: #maInstaller.
      (Smalltalk classNamed: #MaInstaller) new merge: #base.
      (Smalltalk classNamed: #MaBehaviorFinder) browse

 - Chris
 


Best,
Marcel

Am 03.09.2020 15:02:06 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Christoph Thiede

Very interesting ideas here! However, I wonder a) how a stateful SystemNavigation can be kept backward compatible with the current version. And b) how does the StatefulSystemNavigation differ from a MessageTrace in terms of class responsibilities?

Just by the way, I have been dreaming for some time of a version of MessageTrace that has some filter buttons (Material is calling them "chips") on its top that you can use to filter your messages at every time. I'm aware of the existing "filter message list" menu, but I believe the UI can be improved at this point, and filters cannot be reverted again. Some combinable filter buttons like you may knowit from many modern websites (Google, eBay, ...) might be a nice extension IMHO.

Maybe some of all these visions could go into the same tool. Just my 2 cents :-)

> I _wish_ I could write
>      Integer>>#gcd:
> and get a MethodReference instead of a CompiledMethod, since I can easily send #compiledMethod to a MethodReference if I want that (which, I never do, I only ever want the Reference).  I suppose I could implement CompiledMethod>>#asMethodReference, it'd just be so much nicer if it were the other way...

There is already CompiledMethod >> #methodReference, but I see your point. :-) However, we should be cautious with what Kent Beck says in his Best Practices about binary convenience messages (that's at least how I understand it):
"Represent object allocation as a message to one of the arguments to the Complete Creation Method. Add no more than three of these Constructor Methods per system you develop."
While #>>, #>>>, #<< etc. all might be useful shortcuts, they are quite contrary to the idea of natural readability of Smalltalk code because nobody understands them without reading their documentation. It's the same as Python things like "a[-3::-1]" which may be shorter than "(anArray allButLast: 2) reverse" but much more cryptic than the first one on the other hand.

Best,
Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von Chris Muller <[hidden email]>
Gesendet: Dienstag, 29. September 2020 23:51:32
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] The Inbox: Tools-ct.986.mcz
 


On Tue, Sep 29, 2020 at 7:18 AM Eliot Miranda <[hidden email]> wrote:
Hi Marcel,


On Sep 3, 2020, at 6:15 AM, Marcel Taeumel <[hidden email]> wrote:


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.

+1.  Making a SystemNavigation stateful so that it can be assigned something for auto select would be great.  It could also take a pattern not just a string, and hence deal with multi keyword messages better.

If we separate the search and presentation functions there are several benefits too:

- we can get a calculus for search, so that we can have and and or. So something like search for all calls on methods that refer to selector a and selector b in class hierarchy c are easily expressed as the and of three elemental searches, fir selector a, for selector b, for class hierarchy c

- we can get rid of the duplication in the allFoo and browseAllFoo protocols, keeping the search and just sending browse to the result.

- we can derive results in the forms we’d like, so a “navigation” could answer methods, classes, packages, method references, etc

As a suggested implementation Object>>smalltalk or Object>>navigation answers a new SystemSearch instance; SystemSearch instances understand basic queries (allCallsOn:, allImplementorsOf: etc), and an and: and or: protocol that take either another SystemSearch or a message, and answer or update the receiver.  Then at any time one can send autoSelect: to the SystemSearch, and browse to see its results.

BTW, a concise way of creating a message might be cool; Message selector: #allCallsOn: argument: 123 is fine but a little verbose and tedious to type.  #allCallsOn: << { 123 } might be nice, or a little too cryptic.  I don’t know.

+1, I kinda like that!   Although, some objects already implement #<< , hmm...

For years, I've actually been wanting a convenience constructor for MethodReference.  I _wish_ I could write

     Integer>>#gcd:

and get a MethodReference instead of a CompiledMethod, since I can easily send #compiledMethod to a MethodReference if I want that (which, I never do, I only ever want the Reference).  I suppose I could implement CompiledMethod>>#asMethodReference, it'd just be so much nicer if it were the other way...
 
Btw, Andreas hated and I hate typing systemNavigation; so long.  I implement sn in Object as an abbreviation.  Hence my suggestion of smalltalk or navigator above.  search would be even better:

     (self search
          allCallsOn: #systemNavigation) “sets systemNavigation as the default auto select string”
          and: #inPackage: << { #Tools };
          browse

is much more flexible than 
    self sysyemNavigation browseAllCallsOn: #systemNavigation loxalToPackage: #Tools

Or have and: be the default combinatorial so one can cascade ands:

     self search
          allCallsOn: #systemNavigation;
          inPackage: #Tools;
          browse

I like this last one.

A great improvement!  Except #search.  We should stay with #systemNavigation.  We developers have autoComplete, etc., we shouldn't poach Object>>#search from the API namespace from users.

For the ultimate method-selecting power in the universe, MaBehaviorFinder takes the above concept one step further.  Instead of always starting with "all" and filtering, it provides both adding AND removing filters.  So the above example could be:

       self systemNavigation
            addCallsOn: #systemNavigation ;         "adding filter"
            selectPackage: #Tools ;                         "removing filter"
            browse

Or, equally:

       self systemNavigation
            addPackage: #Tools ;                          "adding filter"
            selectCallsOn: #systemNavigation      "removing filter"
            browse

So, various "add" API, ADDS methods to the result, while various "select" API, removes them from the result.  (There's even a third family, "reject" which select the opposite of "select").

There's quite a few methods, but executing them in succession on a stateful instance, as you've described for SystemNavigation here, is exactly how it works, and it's fabulous!   :-D

Just in case anyone is interested in taking a peek at it for inspiration, the following will pop it up:

      Installer new merge: #maInstaller.
      (Smalltalk classNamed: #MaInstaller) new merge: #base.
      (Smalltalk classNamed: #MaBehaviorFinder) browse

 - Chris
 


Best,
Marcel

Am 03.09.2020 15:02:06 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>




Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Eliot Miranda-2
In reply to this post by Chris Muller-3
Hi Chris, 

On Sep 29, 2020, at 2:52 PM, Chris Muller <[hidden email]> wrote:

On Tue, Sep 29, 2020 at 7:18 AM Eliot Miranda <[hidden email]> wrote:
Hi Marcel,

On Sep 3, 2020, at 6:15 AM, Marcel Taeumel <[hidden email]> wrote:


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.

+1.  Making a SystemNavigation stateful so that it can be assigned something for auto select would be great.  It could also take a pattern not just a string, and hence deal with multi keyword messages better.

If we separate the search and presentation functions there are several benefits too:

- we can get a calculus for search, so that we can have and and or. So something like search for all calls on methods that refer to selector a and selector b in class hierarchy c are easily expressed as the and of three elemental searches, fir selector a, for selector b, for class hierarchy c

- we can get rid of the duplication in the allFoo and browseAllFoo protocols, keeping the search and just sending browse to the result.

- we can derive results in the forms we’d like, so a “navigation” could answer methods, classes, packages, method references, etc

As a suggested implementation Object>>smalltalk or Object>>navigation answers a new SystemSearch instance; SystemSearch instances understand basic queries (allCallsOn:, allImplementorsOf: etc), and an and: and or: protocol that take either another SystemSearch or a message, and answer or update the receiver.  Then at any time one can send autoSelect: to the SystemSearch, and browse to see its results.

BTW, a concise way of creating a message might be cool; Message selector: #allCallsOn: argument: 123 is fine but a little verbose and tedious to type.  #allCallsOn: << { 123 } might be nice, or a little too cryptic.  I don’t know.

+1, I kinda like that!   Although, some objects already implement #<< , hmm...

For years, I've actually been wanting a convenience constructor for MethodReference.  I _wish_ I could write

     Integer>>#gcd:

and get a MethodReference instead of a CompiledMethod, since I can easily send #compiledMethod to a MethodReference if I want that (which, I never do, I only ever want the Reference).  I suppose I could implement CompiledMethod>>#asMethodReference, it'd just be so much nicer if it were the other way...

Remember we have unlimited binary selectors now, so I would support you in using either #>>> or #>>>> as a short-hand for deriving a MethodReference.  However, I do think we need to produce a proper suite of references here.  We need ClassReference and BindingReference as well as MethodReference.

 I’ve implemented these when implementing the current closure system a decade ago, wanting to compare similar classes.  What I actually did was hack a diff tool that was a variation on a method list browser that functioned a bit like a change list.  There were MethodReference, ClassReference and TextReference, where TextReference was just a reference whose source was a string/text wrapped by the instance, which could act like either a MethodReference, ClassReference.  The tool MethodDiffTool took two parallel sequences of references and diffed then.  So one could diff two class or method definitions in the system, or diff a class and methods in the system against a source file/package parsed into TextReference.  For completeness we need FooReference for all system objects, so we need them for classes, methods, bindings, and environments.

So go ahead and add >>> or >>>> if that works, but please someone, if possible, round out the reference system so that we have some composable tools, not just browsers.  Being able to diff generically, which is what a reference system allows, is super important.  Right now we have specific tools fir comparing different versions of a method (but not a class nor a binding), and a tool for comparing different versions of a package (& your neat versions extension to the browser), but both of these could be created out of a generic diff tool and suitable reference system, but such an implementation would more easily be extensible to comparing two images, comparing two sources files, etc.  And a system with references for all the semantic categories, methods, classes, bindings and environments, would allow search for references and definitions (general terms for senders and implementors), something I need frequently.

Btw, Andreas hated and I hate typing systemNavigation; so long.  I implement sn in Object as an abbreviation.  Hence my suggestion of smalltalk or navigator above.  search would be even better:

     (self search
          allCallsOn: #systemNavigation) “sets systemNavigation as the default auto select string”
          and: #inPackage: << { #Tools };
          browse

is much more flexible than 
    self sysyemNavigation browseAllCallsOn: #systemNavigation loxalToPackage: #Tools

Or have and: be the default combinatorial so one can cascade ands:

     self search
          allCallsOn: #systemNavigation;
          inPackage: #Tools;
          browse

I like this last one.

A great improvement!  Except #search.  We should stay with #systemNavigation.  We developers have autoComplete, etc., we shouldn't poach Object>>#search from the API namespace from users.

For the ultimate method-selecting power in the universe, MaBehaviorFinder takes the above concept one step further.  Instead of always starting with "all" and filtering, it provides both adding AND removing filters.  So the above example could be:

       self systemNavigation
            addCallsOn: #systemNavigation ;         "adding filter"
            selectPackage: #Tools ;                         "removing filter"
            browse

Or, equally:

       self systemNavigation
            addPackage: #Tools ;                          "adding filter"
            selectCallsOn: #systemNavigation      "removing filter"
            browse

So, various "add" API, ADDS methods to the result, while various "select" API, removes them from the result.  (There's even a third family, "reject" which select the opposite of "select").

Cool (very) but systemNavigation is tooooooo loooooooong.  And unless you show me an application which implements the selector search high up in the class hierarchy I accuse you of the tail wagging the fog turfing me  to  type  s y s t e m N a v I g a t I o n over and over and over again (I got a spam call yesterday afternoon:
     Hello..
     Hello, how are you today sir?
     I’m fine, who is this?
     I’m so and so from such and such
     Ok, and...?
     I want to ask you one question.
     Fire away
     Do you have arthritis?
     No (hangs up)


There's quite a few methods, but executing them in succession on a stateful instance, as you've described for SystemNavigation here, is exactly how it works, and it's fabulous!   :-D

Cool.


Just in case anyone is interested in taking a peek at it for inspiration, the following will pop it up:

      Installer new merge: #maInstaller.
      (Smalltalk classNamed: #MaInstaller) new merge: #base.
      (Smalltalk classNamed: #MaBehaviorFinder) browse

 - Chris
 


Best,
Marcel

Am 03.09.2020 15:02:06 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]>
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 <[hidden email]> 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 ]
>   !
>
>





Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.986.mcz

Chris Muller-4
Hi Eliot,

I'll consider that MethodReference shortcut constructor then.  Say, how about a single bracket, as in:

    MyClass > #theMethodSelector

I like this, because the "Reference" is the highest-level coupling to the object whereas, then, when you want to go deeper (e.g., its CompiledMethod), there is always still the legacy #>>.  And so, if anything, #>>> would go even deeper than that, but I don't know what that would be..

I just checked, looks like it's available.  I hope you enjoy the idea of trying to design our API not merely based on what selectors are "still available", but _crafting_ beautiful API that makes sense.  Just for analogy, a couple of my favorite high-powered API beauties are the Chronology's original Duration constructors (e.g., "1 minute + 30 seconds"), and also that fantastic SortBlock spaceship operator thingy we added a few years back.
 
Cool (very) but systemNavigation is tooooooo loooooooong.  And unless you show me an application which implements the selector search high up in the class hierarchy I accuse you of the tail wagging the fog turfing me  to  type  s y s t e m N a v I g a t I o n over and over and over again (I got a spam call yesterday afternoon:
     Hello..
     Hello, how are you today sir?
     I’m fine, who is this?
     I’m so and so from such and such
     Ok, and...?
     I want to ask you one question.
     Fire away
     Do you have arthritis?
     No (hangs up)

As the first thing I mentioned was code-completion, such an accusation should never have been made.  I don't use any of the luxury (OCompletion, etc.) tools, but, at least in my image, I can type as little as "systemN" and the first hit of Cmd+q is it.  That's just two more keystrokes for something that, if you're finding yourself typing it a lot, maybe it's a sign of a shortcoming in the IDE browsers that should be improved..?  You could also implement #sn as a personal alias..

That's three alternatives to having to type the full "systemNavigation".  IMO, developers are tough, we should leave the best, most-basic words like "search" and "help" and "save", available for external applications.  "Ma-Search" is just such an application -- to search any Object -- I don't think it implements #search exactly, but I do want to implement that for the interactive (argument-free) version of search:.

Regards,
  Chris