If you're needing a decision, I'd vote for option 3.
________________________________ Von: Squeak-dev squeak-dev-bounces@lists.squeakfoundation.org im Auftrag von Taeumel, Marcel Gesendet: Mittwoch, 16. Oktober 2019 11:14:15 An: gettimothy via Squeak-dev Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.123.mcz
Quick suggestion on the formatting. This one:
^ self canBrowseTopic or: [ parentTopic ifNotNil: #canBrowseSubtopic ifNil: [false]]
could become:
^ self canBrowseTopic or: [ parentTopic ifNil: [false] ifNotNil: [:topic | topic canBrowseSubtopic]]
or:
^ self canBrowseTopic or: [parentTopic notNil and: [parentTopic canBrowseSubtopic]]
Hmmm... what are other opinions on this? There is no need for #ifNil/ifNotNil in such a boolean expression?
Best, Marcel
Am 13.10.2019 21:04:19 schrieb commits@source.squeak.org commits@source.squeak.org:
A new version of HelpSystem-Core was added to project The Inbox: http://source.squeak.org/inbox/HelpSystem-Core-ct.123.mcz
==================== Summary ====================
Name: HelpSystem-Core-ct.123 Author: ct Time: 13 October 2019, 9:04:08.373932 pm UUID: dec7ceca-320f-d945-8d2a-c2f6a5e49a52 Ancestors: HelpSystem-Core-ct.120
Refactors HelpBrowser menu: Move menu stuff from HelpBrowser into HelpTopic hierarchy in favor of a better object design
Thanks again, Marcel :-)
=============== Diff against HelpSystem-Core-ct.120 ===============
Item was added: + ----- Method: AbstractHelpTopic>>browseTopicFromParent: (in category 'tools') ----- + browseTopicFromParent: parentTopic + + self canBrowseTopic + ifTrue: [^ self browseTopic]. + parentTopic canBrowseSubtopic + ifTrue: [^ parentTopic browseSubtopic: self]. + !
Item was added: + ----- Method: AbstractHelpTopic>>canBrowseSubtopic (in category 'testing') ----- + canBrowseSubtopic + + ^ false!
Item was added: + ----- Method: AbstractHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ false!
Item was added: + ----- Method: AbstractHelpTopic>>canBrowseTopicFromParent: (in category 'testing') ----- + canBrowseTopicFromParent: parentTopic + + ^ self canBrowseTopic or: [ + parentTopic ifNotNil: #canBrowseSubtopic ifNil: [false]]!
Item was added: + ----- Method: AbstractHelpTopic>>topicMenu:parentTopic: (in category 'menus') ----- + topicMenu: aMenu parentTopic: parentTopic + + aMenu + add: 'Inspect (i)' translated target: self action: #inspect; + add: 'Explore (I)' translated target: self action: #explore. + (self canBrowseTopicFromParent: parentTopic) + ifTrue: [ + aMenu add: 'Browse (b)' translated + target: self + selector: #browseTopicFromParent: + argumentList: {parentTopic} ]. + + ^ aMenu!
Item was added: + ----- Method: AbstractHelpTopic>>topicMenuKey:fromParent: (in category 'menus') ----- + topicMenuKey: aChar fromParent: parentTopic + + aChar + caseOf: { + [$b] -> [(self canBrowseTopicFromParent: parentTopic) + ifTrue: [ self browseTopicFromParent: parentTopic ]]. + [$i] -> [self inspect]. + [$I] -> [self explore] } + otherwise: [^ false]. + ^ true!
Item was added: + ----- Method: ClassAPIHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ true!
Item was added: + ----- Method: ClassBasedHelpTopic>>canBrowseSubtopic (in category 'testing') ----- + canBrowseSubtopic + + ^ true!
Item was added: + ----- Method: ClassBasedHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ true!
Item was added: + ----- Method: DirectoryBasedHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ true!
Item was added: + ----- Method: FileBasedHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ true!
Item was removed: - ----- Method: HelpBrowser>>browseTopic (in category 'actions') ----- - browseTopic - - ^ (self currentTopic respondsTo: #browseTopic) - ifTrue: [self currentTopic browseTopic] - ifFalse: [self currentParentTopic browseSubtopic: self currentTopic]!
Item was removed: - ----- Method: HelpBrowser>>canBrowseTopic (in category 'testing') ----- - canBrowseTopic - - ^ (self currentTopic respondsTo: #browseTopic) - or: [self currentParentTopic respondsTo: #browseSubtopic:]!
Item was removed: - ----- Method: HelpBrowser>>exploreTopic (in category 'actions') ----- - exploreTopic - - ^ self currentTopic explore!
Item was removed: - ----- Method: HelpBrowser>>inspectTopic (in category 'actions') ----- - inspectTopic - - ^ self currentTopic inspect!
Item was changed: ----- Method: HelpBrowser>>treeKey:from:event: (in category 'menus') ----- treeKey: aChar from: aView event: anEvent
anEvent anyModifierKeyPressed ifFalse: [^ false]. + ^ (self currentTopic topicMenuKey: aChar fromParent: self currentParentTopic)! - aChar - caseOf: { - [$b] -> [self browseTopic]. - [$i] -> [self inspectTopic]. - [$I] -> [self exploreTopic]. } - otherwise: [^ false]. - ^ true!
Item was changed: ----- Method: HelpBrowser>>treeListMenu: (in category 'menus') ----- treeListMenu: aMenu
+ ^ self currentTopic + ifNil: [aMenu] + ifNotNil: [:topic | topic + topicMenu: aMenu + parentTopic: self currentParentTopic]! - self currentTopic ifNil: [^ aMenu]. - - aMenu - add: 'Inspect (i)' action: #inspectTopic; - add: 'Explore (I)' action: #exploreTopic. - - self canBrowseTopic ifTrue: [ - aMenu - addLine; - add: 'Browse (b)' action: #browseTopic]. - - ^ aMenu!
Item was added: + ----- Method: MethodListHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ true!
Item was added: + ----- Method: PackageAPIHelpTopic>>canBrowseTopic (in category 'testing') ----- + canBrowseTopic + + ^ true!