The Trunk: Morphic-mt.1347.mcz

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

The Trunk: Morphic-mt.1347.mcz

commits-2
Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-mt.1347.mcz

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

Name: Morphic-mt.1347
Author: mt
Time: 18 July 2017, 10:11:56.69381 am
UUID: f95fc4b5-03e5-2f45-9a3f-087fb10cae98
Ancestors: Morphic-eem.1346

Regarding window colors and window listing, improve robustness for models that do not subclass Model and forget to provide #windowColorToUse.

Note that we could have added that message to Object but I do prefer not to clutter the interface any further.

=============== Diff against Morphic-eem.1346 ===============

Item was changed:
  ----- Method: TheWorldMainDockingBar>>listWindowsOn: (in category 'submenu - windows') -----
  listWindowsOn: menu
 
  | windows |
  windows := self allVisibleWindows sorted: [:winA :winB |
  ((winA model isNil or: [winB model isNil]) or: [winA model name = winB model name])
  ifTrue: [winA label < winB label]
  ifFalse: [winA model name < winB model name]].
  windows ifEmpty: [
  menu addItem: [ :item |
  item
  contents: 'No Windows' translated;
  isEnabled: false ] ].
  windows do: [ :each |
+ | windowColor |
+ windowColor := (each model respondsTo: #windowColorToUse)
+ ifTrue: [each model windowColorToUse]
+ ifFalse: [UserInterfaceTheme current get: #uniformWindowColor for: Model].
  menu addItem: [ :item |
  item
  contents: (self windowMenuItemLabelFor: each);
+ icon: (self colorIcon: windowColor);
- icon: (each model ifNotNil: [self colorIcon: each model windowColorToUse]);
  target: each;
  selector: #comeToFront;
  subMenuUpdater: self
  selector: #windowMenuFor:on:
  arguments: { each };
  action: [ each beKeyWindow; expand ] ] ].
  menu
  addLine;
  add: 'Close all windows' target: self selector: #closeAllWindowsUnsafe;
  addItem: [:item | item
  contents: 'Close all windows without changes';
  target: self;
  icon: MenuIcons smallBroomIcon;
  selector: #closeAllWindows];
  add: 'Close all windows but workspaces' target: self selector: #closeAllWindowsButWorkspaces.!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-mt.1347.mcz

Eliot Miranda-2
nice!

On Tue, Jul 18, 2017 at 1:12 AM, <[hidden email]> wrote:
Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-mt.1347.mcz

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

Name: Morphic-mt.1347
Author: mt
Time: 18 July 2017, 10:11:56.69381 am
UUID: f95fc4b5-03e5-2f45-9a3f-087fb10cae98
Ancestors: Morphic-eem.1346

Regarding window colors and window listing, improve robustness for models that do not subclass Model and forget to provide #windowColorToUse.

Note that we could have added that message to Object but I do prefer not to clutter the interface any further.

=============== Diff against Morphic-eem.1346 ===============

Item was changed:
  ----- Method: TheWorldMainDockingBar>>listWindowsOn: (in category 'submenu - windows') -----
  listWindowsOn: menu

        | windows |
        windows := self allVisibleWindows sorted: [:winA :winB |
                ((winA model isNil or: [winB model isNil]) or: [winA model name = winB model name])
                        ifTrue: [winA label < winB label]
                        ifFalse: [winA model name < winB model name]].
        windows ifEmpty: [
                menu addItem: [ :item |
                        item
                                contents: 'No Windows' translated;
                                isEnabled: false ] ].
        windows do: [ :each |
+               | windowColor |
+               windowColor := (each model respondsTo: #windowColorToUse)
+                       ifTrue: [each model windowColorToUse]
+                       ifFalse: [UserInterfaceTheme current get: #uniformWindowColor for: Model].
                menu addItem: [ :item |
                        item
                                contents: (self windowMenuItemLabelFor: each);
+                               icon: (self colorIcon: windowColor);
-                               icon: (each model ifNotNil: [self colorIcon: each model windowColorToUse]);
                                target: each;
                                selector: #comeToFront;
                                subMenuUpdater: self
                                selector: #windowMenuFor:on:
                                arguments: { each };
                                action: [ each beKeyWindow; expand ] ] ].
        menu
                addLine;
                add: 'Close all windows' target: self selector: #closeAllWindowsUnsafe;
                addItem: [:item | item
                        contents: 'Close all windows without changes';
                        target: self;
                        icon: MenuIcons smallBroomIcon;
                        selector: #closeAllWindows];
                add: 'Close all windows but workspaces' target: self selector: #closeAllWindowsButWorkspaces.!





--
_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-mt.1347.mcz

Bert Freudenberg
How about using onDNU: instead of respondsTo:?

    windowColor := [each model windowColorToUse] onDNU: #windowColorToUse do:
        [UserInterfaceTheme current get: #uniformWindowColor for: Model].

I don't have a strong opinion, but it feels "cleaner".

- Bert -

On Tue, Jul 18, 2017 at 9:14 PM, Eliot Miranda <[hidden email]> wrote:
nice!

On Tue, Jul 18, 2017 at 1:12 AM, <[hidden email]> wrote:
Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-mt.1347.mcz

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

Name: Morphic-mt.1347
Author: mt
Time: 18 July 2017, 10:11:56.69381 am
UUID: f95fc4b5-03e5-2f45-9a3f-087fb10cae98
Ancestors: Morphic-eem.1346

Regarding window colors and window listing, improve robustness for models that do not subclass Model and forget to provide #windowColorToUse.

Note that we could have added that message to Object but I do prefer not to clutter the interface any further.

=============== Diff against Morphic-eem.1346 ===============

Item was changed:
  ----- Method: TheWorldMainDockingBar>>listWindowsOn: (in category 'submenu - windows') -----
  listWindowsOn: menu

        | windows |
        windows := self allVisibleWindows sorted: [:winA :winB |
                ((winA model isNil or: [winB model isNil]) or: [winA model name = winB model name])
                        ifTrue: [winA label < winB label]
                        ifFalse: [winA model name < winB model name]].
        windows ifEmpty: [
                menu addItem: [ :item |
                        item
                                contents: 'No Windows' translated;
                                isEnabled: false ] ].
        windows do: [ :each |
+               | windowColor |
+               windowColor := (each model respondsTo: #windowColorToUse)
+                       ifTrue: [each model windowColorToUse]
+                       ifFalse: [UserInterfaceTheme current get: #uniformWindowColor for: Model].
                menu addItem: [ :item |
                        item
                                contents: (self windowMenuItemLabelFor: each);
+                               icon: (self colorIcon: windowColor);
-                               icon: (each model ifNotNil: [self colorIcon: each model windowColorToUse]);
                                target: each;
                                selector: #comeToFront;
                                subMenuUpdater: self
                                selector: #windowMenuFor:on:
                                arguments: { each };
                                action: [ each beKeyWindow; expand ] ] ].
        menu
                addLine;
                add: 'Close all windows' target: self selector: #closeAllWindowsUnsafe;
                addItem: [:item | item
                        contents: 'Close all windows without changes';
                        target: self;
                        icon: MenuIcons smallBroomIcon;
                        selector: #closeAllWindows];
                add: 'Close all windows but workspaces' target: self selector: #closeAllWindowsButWorkspaces.!





--
_,,,^..^,,,_
best, Eliot






Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Morphic-mt.1347.mcz

marcel.taeumel
Bert Freudenberg wrote
How about using onDNU: instead of respondsTo:?

    windowColor := [each model windowColorToUse] onDNU: #windowColorToUse
 do:
        [UserInterfaceTheme current get: #uniformWindowColor for: Model].

I don't have a strong opinion, but it feels "cleaner".

- Bert -

On Tue, Jul 18, 2017 at 9:14 PM, Eliot Miranda <[hidden email]>
wrote:

> nice!
>
> On Tue, Jul 18, 2017 at 1:12 AM, <[hidden email]> wrote:
>
>> Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
>> http://source.squeak.org/trunk/Morphic-mt.1347.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Morphic-mt.1347
>> Author: mt
>> Time: 18 July 2017, 10:11:56.69381 am
>> UUID: f95fc4b5-03e5-2f45-9a3f-087fb10cae98
>> Ancestors: Morphic-eem.1346
>>
>> Regarding window colors and window listing, improve robustness for models
>> that do not subclass Model and forget to provide #windowColorToUse.
>>
>> Note that we could have added that message to Object but I do prefer not
>> to clutter the interface any further.
>>
>> =============== Diff against Morphic-eem.1346 ===============
>>
>> Item was changed:
>>   ----- Method: TheWorldMainDockingBar>>listWindowsOn: (in category
>> 'submenu - windows') -----
>>   listWindowsOn: menu
>>
>>         | windows |
>>         windows := self allVisibleWindows sorted: [:winA :winB |
>>                 ((winA model isNil or: [winB model isNil]) or: [winA
>> model name = winB model name])
>>                         ifTrue: [winA label < winB label]
>>                         ifFalse: [winA model name < winB model name]].
>>         windows ifEmpty: [
>>                 menu addItem: [ :item |
>>                         item
>>                                 contents: 'No Windows' translated;
>>                                 isEnabled: false ] ].
>>         windows do: [ :each |
>> +               | windowColor |
>> +               windowColor := (each model respondsTo: #windowColorToUse)
>> +                       ifTrue: [each model windowColorToUse]
>> +                       ifFalse: [UserInterfaceTheme current get:
>> #uniformWindowColor for: Model].
>>                 menu addItem: [ :item |
>>                         item
>>                                 contents: (self windowMenuItemLabelFor:
>> each);
>> +                               icon: (self colorIcon: windowColor);
>> -                               icon: (each model ifNotNil: [self
>> colorIcon: each model windowColorToUse]);
>>                                 target: each;
>>                                 selector: #comeToFront;
>>                                 subMenuUpdater: self
>>                                 selector: #windowMenuFor:on:
>>                                 arguments: { each };
>>                                 action: [ each beKeyWindow; expand ] ] ].
>>         menu
>>                 addLine;
>>                 add: 'Close all windows' target: self selector:
>> #closeAllWindowsUnsafe;
>>                 addItem: [:item | item
>>                         contents: 'Close all windows without changes';
>>                         target: self;
>>                         icon: MenuIcons smallBroomIcon;
>>                         selector: #closeAllWindows];
>>                 add: 'Close all windows but workspaces' target: self
>> selector: #closeAllWindowsButWorkspaces.!
>>
>>
>>
>
>
> --
> _,,,^..^,,,_
> best, Eliot
>
>
>
>
Hi Bert,

I've never heard of this message before. :) This would be an option but it impedes readability because of the DNU acronym, which could have been MNU btw. ;)

I would rather call it #ifUnknown:do: or do something similar to #perform:orSendTo:? Hmm... well, using/handling/expecting exceptions is quite different from asking an object whether it responds to a message. Basic object messaging and exception handling are two concepts on different abstraction levels. At least for me. Then, the DNU exception is generic and not domain-specific. I would prefer to use exception handling with domain-specific exceptions in such specific (i.e. windows, colors, etc.) code.

TL;DR: I prefer this respondsTo-check-pattern over the DNU-handler-pattern because of code readability.

Best,
Marcel