The Inbox: System-ct.1158.mcz

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

The Inbox: System-ct.1158.mcz

commits-2
Christoph Thiede uploaded a new version of System to project The Inbox:
http://source.squeak.org/inbox/System-ct.1158.mcz

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

Name: System-ct.1158
Author: ct
Time: 16 May 2020, 8:53:15.785708 pm
UUID: 03cddc8b-5593-504d-95c0-fc6851a15d88
Ancestors: System-nice.1157

Refactor AppRegistry class >> #askForDefault. Add multilingual support. If request is cancelled by the user, don't set a default and return a fallback value.

=============== Diff against System-nice.1157 ===============

Item was changed:
  ----- Method: AppRegistry class>>askForDefault (in category 'defaults') -----
  askForDefault
 
+ self requestDefault ifNotNil: [:newDefault |
+ default := newDefault].
+ ^ default ifNil: [self registeredClasses first]!
- self registeredClasses isEmpty ifTrue:
- [self inform: 'There are no ', self appName, ' applications registered.'.
- ^ default := nil].
- self registeredClasses size = 1 ifTrue:
- [^ default := self registeredClasses anyOne].
- default :=  UIManager default
- chooseFrom: (self registeredClasses collect: [:ea | ea nameForViewer])
- values: self registeredClasses
- title: 'Which ', self appName, ' would you prefer?'.
- default ifNil: [default := self registeredClasses first].
- ^default.!

Item was added:
+ ----- Method: AppRegistry class>>requestDefault (in category 'defaults') -----
+ requestDefault
+
+ self registeredClasses isEmpty ifTrue: [
+ self inform: ('There are no {1} applications registered' translated format: {self appName}).
+ ^ nil].
+ self registeredClasses size = 1 ifTrue: [
+ ^ self registeredClasses anyOne].
+ ^ UIManager default
+ chooseFrom: (self registeredClasses collect: [:ea | ea nameForViewer])
+ values: self registeredClasses
+ title: ('Which {1} would you prefer?' translated format: {self appName})!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1158.mcz

marcel.taeumel
Hi Christoph.

- What about ToolSet class >> #askForDefault?
- What is the benefit of extracting #requestDefault out of #askDefault? It is not obvious that one has a side effect and the other does not.
UIManager default -> Project uiManager

Best,
Marcel

Am 16.05.2020 20:53:34 schrieb [hidden email] <[hidden email]>:

Christoph Thiede uploaded a new version of System to project The Inbox:
http://source.squeak.org/inbox/System-ct.1158.mcz

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

Name: System-ct.1158
Author: ct
Time: 16 May 2020, 8:53:15.785708 pm
UUID: 03cddc8b-5593-504d-95c0-fc6851a15d88
Ancestors: System-nice.1157

Refactor AppRegistry class >> #askForDefault. Add multilingual support. If request is cancelled by the user, don't set a default and return a fallback value.

=============== Diff against System-nice.1157 ===============

Item was changed:
----- Method: AppRegistry class>>askForDefault (in category 'defaults') -----
askForDefault

+ self requestDefault ifNotNil: [:newDefault |
+ default := newDefault].
+ ^ default ifNil: [self registeredClasses first]!
- self registeredClasses isEmpty ifTrue:
- [self inform: 'There are no ', self appName, ' applications registered.'.
- ^ default := nil].
- self registeredClasses size = 1 ifTrue:
- [^ default := self registeredClasses anyOne].
- default := UIManager default
- chooseFrom: (self registeredClasses collect: [:ea | ea nameForViewer])
- values: self registeredClasses
- title: 'Which ', self appName, ' would you prefer?'.
- default ifNil: [default := self registeredClasses first].
- ^default.!

Item was added:
+ ----- Method: AppRegistry class>>requestDefault (in category 'defaults') -----
+ requestDefault
+
+ self registeredClasses isEmpty ifTrue: [
+ self inform: ('There are no {1} applications registered' translated format: {self appName}).
+ ^ nil].
+ self registeredClasses size = 1 ifTrue: [
+ ^ self registeredClasses anyOne].
+ ^ UIManager default
+ chooseFrom: (self registeredClasses collect: [:ea | ea nameForViewer])
+ values: self registeredClasses
+ title: ('Which {1} would you prefer?' translated format: {self appName})!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1158.mcz

Christoph Thiede

Hi Marcel,


What about ToolSet class >> #askForDefault?


Oh, that's some duplication. Maybe we can just delete that override?

What is the benefit of extracting #requestDefault out of #askDefault? It is not obvious that one has a side effect and the other does not.

I found that this could factor out the side effect. IMHO it is easier to see where the default is actually set if this happens at one single place.
In addition, if the user cancels the dialog, #askForDefault should return a fallback value, but it should *not* store this fallback as the default!

UIManager default -> Project uiManager

Good point, will update in the next revision :)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Sonntag, 17. Mai 2020 14:35:24
An: gettimothy via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1158.mcz
 
Hi Christoph.

- What about ToolSet class >> #askForDefault?
- What is the benefit of extracting #requestDefault out of #askDefault? It is not obvious that one has a side effect and the other does not.
UIManager default -> Project uiManager

Best,
Marcel

Am 16.05.2020 20:53:34 schrieb [hidden email] <[hidden email]>:

Christoph Thiede uploaded a new version of System to project The Inbox:
http://source.squeak.org/inbox/System-ct.1158.mcz

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

Name: System-ct.1158
Author: ct
Time: 16 May 2020, 8:53:15.785708 pm
UUID: 03cddc8b-5593-504d-95c0-fc6851a15d88
Ancestors: System-nice.1157

Refactor AppRegistry class >> #askForDefault. Add multilingual support. If request is cancelled by the user, don't set a default and return a fallback value.

=============== Diff against System-nice.1157 ===============

Item was changed:
----- Method: AppRegistry class>>askForDefault (in category 'defaults') -----
askForDefault

+ self requestDefault ifNotNil: [:newDefault |
+ default := newDefault].
+ ^ default ifNil: [self registeredClasses first]!
- self registeredClasses isEmpty ifTrue:
- [self inform: 'There are no ', self appName, ' applications registered.'.
- ^ default := nil].
- self registeredClasses size = 1 ifTrue:
- [^ default := self registeredClasses anyOne].
- default := UIManager default
- chooseFrom: (self registeredClasses collect: [:ea | ea nameForViewer])
- values: self registeredClasses
- title: 'Which ', self appName, ' would you prefer?'.
- default ifNil: [default := self registeredClasses first].
- ^default.!

Item was added:
+ ----- Method: AppRegistry class>>requestDefault (in category 'defaults') -----
+ requestDefault
+
+ self registeredClasses isEmpty ifTrue: [
+ self inform: ('There are no {1} applications registered' translated format: {self appName}).
+ ^ nil].
+ self registeredClasses size = 1 ifTrue: [
+ ^ self registeredClasses anyOne].
+ ^ UIManager default
+ chooseFrom: (self registeredClasses collect: [:ea | ea nameForViewer])
+ values: self registeredClasses
+ title: ('Which {1} would you prefer?' translated format: {self appName})!