The Inbox: HelpSystem-Core-ct.125.mcz

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

The Inbox: HelpSystem-Core-ct.125.mcz

commits-2
A new version of HelpSystem-Core was added to project The Inbox:
http://source.squeak.org/inbox/HelpSystem-Core-ct.125.mcz

==================== Summary ====================

Name: HelpSystem-Core-ct.125
Author: ct
Time: 14 October 2019, 12:33:57.997932 am
UUID: bdc17b48-7f21-844e-ad05-f3ba9876fd3f
Ancestors: HelpSystem-Core-mt.116

Adds CustomHelp>>#defaultPageKey to show the contents of a specified subtopic if a ClassBasedHelpTopic is selected

=============== Diff against HelpSystem-Core-mt.116 ===============

Item was changed:
  ----- Method: ClassBasedHelpTopic>>contents (in category 'accessing') -----
  contents
  "A book has no contents. Only its pages do."
 
+ ^ self defaultSubtopic ifNil: [''] ifNotNil: #contents!
- ^ ''!

Item was added:
+ ----- Method: ClassBasedHelpTopic>>defaultSubtopic (in category 'accessing') -----
+ defaultSubtopic
+
+ ^ self helpClass defaultPageKey
+ ifNotNil: [:key |
+ self subtopics
+ detect: [:topic | topic key = key]
+ ifNone: [nil]]
+ ifNil: [nil]!

Item was added:
+ ----- Method: ClassBasedHelpTopic>>key (in category 'accessing') -----
+ key
+
+ ^ self helpClass!

Item was added:
+ ----- Method: CustomHelp class>>defaultPageKey (in category 'accessing') -----
+ defaultPageKey
+ "Key of the page to show if no subtopic is selected, or nil"
+ ^ nil!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: HelpSystem-Core-ct.125.mcz

Tobias Pape
Hi

I'd like to propose a little change:


> On 14.10.2019, at 00:34, [hidden email] wrote:
>
>  ----- Method: ClassBasedHelpTopic>>contents (in category 'accessing') -----
>  contents
>   "A book has no contents. Only its pages do."
>  
> + ^ self defaultSubtopic ifNil: [''] ifNotNil: #contents!

        ^ self defaultSubtopic
                ifNotNil: [:topic | topic contents]
                ifNil: ['']

This changes the order so that the non-exceptional case is first. It also avoids the meta-level.


> - ^ ''!
>
> Item was added:
> + ----- Method: ClassBasedHelpTopic>>defaultSubtopic (in category 'accessing') -----
> + defaultSubtopic
> +
> + ^ self helpClass defaultPageKey
> + ifNotNil: [:key |
> + self subtopics
> + detect: [:topic | topic key = key]
> + ifNone: [nil]]
> + ifNil: [nil]!

I'd leave out the 'ifNil: [nil]' case; it's implied when sending ifNotNil:.

Best regards
        -Tobias

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: HelpSystem-Core-ct.125.mcz

Christoph Thiede

Yes, you're right :)

You find the use of Symbol>>#value: "too meta"? I like it as a good readable shorthand.


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Tobias Pape <[hidden email]>
Gesendet: Montag, 14. Oktober 2019 09:16:21
An: [hidden email]
Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
 
Hi

I'd like to propose a little change:


> On 14.10.2019, at 00:34, [hidden email] wrote:
>
>  ----- Method: ClassBasedHelpTopic>>contents (in category 'accessing') -----
>  contents
>        "A book has no contents. Only its pages do."
>       
> +      ^ self defaultSubtopic ifNil: [''] ifNotNil: #contents!

        ^ self defaultSubtopic
                ifNotNil: [:topic | topic contents]
                ifNil: ['']

This changes the order so that the non-exceptional case is first. It also avoids the meta-level.


> -      ^ ''!
>
> Item was added:
> + ----- Method: ClassBasedHelpTopic>>defaultSubtopic (in category 'accessing') -----
> + defaultSubtopic
> +
> +      ^ self helpClass defaultPageKey
> +              ifNotNil: [:key |
> +                      self subtopics
> +                              detect: [:topic | topic key = key]
> +                              ifNone: [nil]]
> +              ifNil: [nil]!

I'd leave out the 'ifNil: [nil]' case; it's implied when sending ifNotNil:.

Best regards
        -Tobias



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: HelpSystem-Core-ct.125.mcz

marcel.taeumel
Hi Christoph. :-)

 I like it as a good readable shorthand.

That's debatable. ;-) "#contents" is a Symbol. "[:topic | topic contents]" is a block with message sends. I find the latter more readable because it reveals the message sends. You can debug it, put a "self halt" in it, etc.

 self defaultSubtopic ifNil: [''] ifNotNil: #contents

I don't like the asymmetry between block and symbol. So, in this case, I prefer blocks over that symbol shorthand for a second reason.

Best,
Marcel

Am 14.10.2019 11:55:39 schrieb Thiede, Christoph <[hidden email]>:

Yes, you're right :)

You find the use of Symbol>>#value: "too meta"? I like it as a good readable shorthand.


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Tobias Pape <[hidden email]>
Gesendet: Montag, 14. Oktober 2019 09:16:21
An: [hidden email]
Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
 
Hi

I'd like to propose a little change:


> On 14.10.2019, at 00:34, [hidden email] wrote:
>
>  ----- Method: ClassBasedHelpTopic>>contents (in category 'accessing') -----
>  contents
>        "A book has no contents. Only its pages do."
>       
> +      ^ self defaultSubtopic ifNil: [''] ifNotNil: #contents!

        ^ self defaultSubtopic
                ifNotNil: [:topic | topic contents]
                ifNil: ['']

This changes the order so that the non-exceptional case is first. It also avoids the meta-level.


> -      ^ ''!
>
> Item was added:
> + ----- Method: ClassBasedHelpTopic>>defaultSubtopic (in category 'accessing') -----
> + defaultSubtopic
> +
> +      ^ self helpClass defaultPageKey
> +              ifNotNil: [:key |
> +                      self subtopics
> +                              detect: [:topic | topic key = key]
> +                              ifNone: [nil]]
> +              ifNil: [nil]!

I'd leave out the 'ifNil: [nil]' case; it's implied when sending ifNotNil:.

Best regards
        -Tobias



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: HelpSystem-Core-ct.125.mcz

Christoph Thiede

Well, should I commit again, or is it easier for the merger to align the code style?


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 15. Oktober 2019 15:23:54
An: gettimothy via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
 
Hi Christoph. :-)

 I like it as a good readable shorthand.

That's debatable. ;-) "#contents" is a Symbol. "[:topic | topic contents]" is a block with message sends. I find the latter more readable because it reveals the message sends. You can debug it, put a "self halt" in it, etc.

 self defaultSubtopic ifNil: [''] ifNotNil: #contents

I don't like the asymmetry between block and symbol. So, in this case, I prefer blocks over that symbol shorthand for a second reason.

Best,
Marcel

Am 14.10.2019 11:55:39 schrieb Thiede, Christoph <[hidden email]>:

Yes, you're right :)

You find the use of Symbol>>#value: "too meta"? I like it as a good readable shorthand.


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Tobias Pape <[hidden email]>
Gesendet: Montag, 14. Oktober 2019 09:16:21
An: [hidden email]
Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.125.mcz
 
Hi

I'd like to propose a little change:


> On 14.10.2019, at 00:34, [hidden email] wrote:
>
>  ----- Method: ClassBasedHelpTopic>>contents (in category 'accessing') -----
>  contents
>        "A book has no contents. Only its pages do."
>       
> +      ^ self defaultSubtopic ifNil: [''] ifNotNil: #contents!

        ^ self defaultSubtopic
                ifNotNil: [:topic | topic contents]
                ifNil: ['']

This changes the order so that the non-exceptional case is first. It also avoids the meta-level.


> -      ^ ''!
>
> Item was added:
> + ----- Method: ClassBasedHelpTopic>>defaultSubtopic (in category 'accessing') -----
> + defaultSubtopic
> +
> +      ^ self helpClass defaultPageKey
> +              ifNotNil: [:key |
> +                      self subtopics
> +                              detect: [:topic | topic key = key]
> +                              ifNone: [nil]]
> +              ifNil: [nil]!

I'd leave out the 'ifNil: [nil]' case; it's implied when sending ifNotNil:.

Best regards
        -Tobias