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 <squeak-dev-bounces at lists.squeakfoundation.org> 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 commits at source.squeak.org <commits at source.squeak.org>:

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})!


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200517/a09addc1/attachment.html>