Oh my god! :) 2016-05-04 10:12 GMT+02:00 Peter Uhnák <i.uhnak@gmail.com>:

Yes, let's discuss why ButtonModel needs 36 instance variables, and TextInputFieldModel 49. :)

On Wed, May 4, 2016 at 9:51 AM, Denis Kudriashov <dionisiydk@gmail.com> wrote:

...

Hi.

I want to join

2016-05-04 8:19 GMT+02:00 stepharo <stepharo@free.fr>:

...

Hi guys

this afternoon we will have a meeting with Peter and others to talk about cleaning Spec API.

If you want/can join let us know

else we can organise another skype.

if you have ideas let us know too.

Stef

by a factor of at least 2,5x compared to pure Morphic (I've seen a 5x).

This doesn't mean anything. Can you please provide concrete numbers (what you experience) and what is your expected target? Because I do not have such problematic experience. Even if morphic took 40 ms and Spec 100 ms to open a window, then it's a non-priority problem (~100ms to open a medium-sized window is a non-issue). Unless you are opening such windows every minute… Also I am a bit afraid that the slowness would come from the layout processing / composing the objects… which is a really complex piece, and right now our priority is cleaning up the user-facing API. But at least we can think about it, whether we would be able to do something about it now or not. Peter

First we should clean it, then we speed it up. Stef Le 4/5/16 à 15:45, Stephan Eggermont a écrit :

On 04/05/16 13:43, Peter Uhnák wrote:

...

...

by a factor of at least 2,5x compared to pure Morphic (I've seen a 5x).

This doesn't mean anything. Can you please provide concrete numbers (what you experience) and what is your expected target? Because I do not have such problematic experience.

Thierry develops on a machine that is somewhat slower than those most Pharo users develop on: a Celeron 2955U machine. For Pharo I'd hope to be able to reasonably do development on a Raspberry Pi2.

He is very aware of Morphic performance issues, so that factor 2.5 is not the layout processing, as that is the same for Morphic and Spec.

We could add some Spec performance tests to jenkins/travis to see when we have regressions.

Stephan

2016-05-04 13:43 GMT+02:00 Peter Uhnák <i.uhnak@gmail.com>:

by a factor of at least 2,5x compared to pure Morphic (I've seen a 5x).

...

This doesn't mean anything. Can you please provide concrete numbers (what you experience) and what is your expected target? Because I do not have such problematic experience.

It depends a lot on the underlying platform you use. If you are on a Macbook Pro with a core i7 with turbo boost of 3GHz, no Pharo GUI, even very badly coded, will register. On a lower spec processor, it is significantly different.

Even if morphic took 40 ms and Spec 100 ms to open a window, then it's a non-priority problem (~100ms to open a medium-sized window is a non-issue). Unless you are opening such windows every minute…

I'm talking about 250ms to open a very small window, 400/500ms for some tools, and a significant delay on many interactions because you have to wait for the rebuilding of a window components. It becomes problematic when you can type faster than the GUI reacts (scrolling with keypresses, for example).

Also I am a bit afraid that the slowness would come from the layout processing / composing the objects… which is a really complex piece, and right now our priority is cleaning up the user-facing API.

When profiling, it doesn't seems to be the layout. But processing the object spec, yes.

But at least we can think about it, whether we would be able to do something about it now or not.

Thanks, Thierry

Peter

On 04/05/16 15:24, stepharo wrote:

We are thinking that we would like to rename ComposableModel into Presenter

to convey its role into the MVC triad.

What do the spec users think?

There are too many Models. A clear distinction between DomainModel and ViewModel is needed. Please explain when a ViewController should be a Presenter. Where is the boundary between MVC and MVP? Stephan

On 04/05/16 16:10, Denis Kudriashov wrote:

2016-05-04 15:34 GMT+02:00 Stephan Eggermont <stephan@stack.nl>:

...

There are too many Models. A clear distinction between DomainModel and ViewModel is needed. Please explain when a ViewController should be a Presenter. Where is the boundary between MVC and MVP?

But there is good article about it http://www.object-arts.com/downloads/papers/TwistingTheTriad.PDF

I know. But that is not what Spec does. Stephan

On 04/05/16 18:18, stepharo wrote:

Can you open bug entries with such points?

18164 Put drag-and-drop behaviour in one object 18165 ComposableModel: aboutText seems to be in the wrong place 18166 Where should focus change keystrokes be handled? Stephan

Hi, today with Stef we spent some time going through Spec. But first to address some points In AbstractWidgetModel we have

dragEnabled dropEnabled dragTransformationBlock wantDropBlock acceptDropBlock transferBlock

That needs to be one instance variable dealing with drag and drop.

We question whether this belongs to AbstractWidgetModel, as only three (TreeModel, ListModel, and TabModel (iirc)) benefit from it in any way. One option would be to have an intermediate subclass so we can move at least some of it, like this ​ As for separating into a separate instance variable… I agree from design perspective… however having directly available methods is very user-friendly when you are configuring it, because you can use a cascade. ^ TreeModel new roots: #(a b c d); childrenBlock: [:it | it = #c ifTrue: [ {1. 2. 3} ] ifFalse: [ {} ]]; dragEnabled: true; dropEnabled: true. Which is quite nice. Without it you would have to do (tm := TreeModel new) roots :#(..); childrenBlock: [ :it | ... ]. tm dragAndDrop dragEnabled: true. tm dragAndDrop dropEnabled: true. There are different ways how to have it separate and provide and nice-ish API, for example keep the messages, but delegate the behavior. Or provide a configuration block. This is for example used when you are configuring a MenuModel/MenuGroup/MenuItem. ^ TreeModel new roots: #(...); childrenBlock: [ :it | ... ]; dragAndDrop: [ :dnd | dnd dragEnabled: true; dropEnabled: true ] The same questions also applies to Styles (color, borderColor, borderWidth). Which I would prefer to remove altogether (it's not really used, and I question whether whether it even belongs to Spec side). If we want to keep it, then separating it would be a nice idea (I've started, slice in Pharo60Inbox) ComposableModel: aboutText seems to be in the wrong place.

That is a WindowModel property, and we seem to need some kind of mechanism to propagate property access in the widget hierarchy.

I though so too at first, however this is very same as #title. In fact having directly in ComposableModel is very convenient. MyFormEditor>>title ^ 'Form Editor' MyFormEditor>>aboutText ^ 'Neither #aboutText, nor #title is actually used by MyFormEditor, but by WindowModel, but it makes for nicer API' ComposableModel: keyStrokesForNextFocusHolder keyStrokesForPreviousFocusHolder

I'm pretty sure I do not want

focus switch to be an individual widget concern Take a look at http://smalltalkhub.com/#!/~StephaneDucasse/Deprecator We started cleaning up the API a bit. The idea is to start from Spec side, figure out what we need and then adapt adaptor… and not try to reimplement Morphic. Right now only ButtonModel (ButtonPresenter) is done, but without shortcuts. We would like to introduce some common/uniform behavior for this instead of each Widget redoing it in a chaos. And some more cleanup sits in Pharo60Inbox... Peter On Thu, May 5, 2016 at 1:18 PM, Thierry Goubier <thierry.goubier@gmail.com> wrote:

Le 04/05/2016 19:18, Stephan Eggermont a écrit :

...

On 04/05/16 18:18, stepharo wrote:

...

Can you open bug entries with such points?

18164 Put drag-and-drop behaviour in one object

18165 ComposableModel: aboutText seems to be in the wrong place

18166 Where should focus change keystrokes be handled?

For this one there was an debate about the following point: why Spec needs to reimplement the focus change keystroke implemented by Morphic?

Thierry

Stephan

...

On Sun, May 8, 2016 at 4:28 PM, Stephan Eggermont <stephan@stack.nl> wrote:

On 07-05-16 22:04, Peter Uhnák wrote:

...

We question whether this belongs to AbstractWidgetModel, as only > three (TreeModel, ListModel, and TabModel (iirc)) benefit from it in

any way.

ContainerModel, DropListModel, ImageModel, LabelModel, MenuModel, TextModel and WindowModel.

So no.

Can you give me an example of how would you use it with LabelModel?

The same questions also > applies to Styles (color, borderColor,

...

borderWidth). Which I would >

prefer to remove altogether (it's not really used, and I question > whether whether it even belongs to Spec side).

Where would you like to define application style default overrides then? People ask about this all the time, and the reason it's used so little is because our implementation in the widgets is so bad: very inconsistent naming and default styles ignoring overridden values

If we separate it, then we can think about how to apply it properly. Right now it's not really possible to unify.

Le 4/5/16 à 19:18, Stephan Eggermont a écrit :

On 04/05/16 18:18, stepharo wrote:

...

Can you open bug entries with such points?

18164 Put drag-and-drop behaviour in one object

We were looking and I proposed to AbstractWidgetModel ButtonModel CheckboxModel DragDropModel TabModel TreeModel ListModel

18165 ComposableModel: aboutText seems to be in the wrong place

peter check and this is ok because like that a composable model can do the same as for title just declare aboutText and the window grab it.

18166 Where should focus change keystrokes be handled?

Stephan

Le 4/5/16 à 21:26, Glenn Cavarlé a écrit :

stepharo wrote

...

cleaning Spec API. +10 ! just general remarks i have about Spec: Glenn send code :)

I think the role of Spec may be clarified. If Spec is just an UI builder we may remove the "overengineering" and we need another framework that provides someting like MVC, MVP, MVVM or ... If Spec intent to be an "app" framework, it is too confused now, and yes, it needs a clear separation between the presentation, the presentation logic, the domain logic, ... It is the second Spec should be like MVP

Maybe, extending the SpecLayout's responsibility should be one idea to handle the full "presentation" part, like the role played by markup languages.

Yes this is one idea

I'm not saying I want to transform Spec to XML :), but its role is basically the same.

Indeed

The existing separation between the widget declarations (instance side) and the widgets composition (class side) seem confusing to me.

Totally. If you look in Deprecator (sorry bad name) you will see a new ButtonPresenter API and tests We moved all the Model adaption code into the Adapter. The goal is to remove the Adapter at runtime but for now we need better widgets.

And we need also a clearest way to bind domain models to widgets. Using #whenSomethingChangedDo:aBlock everywhere make code a bit hard to read and to debug, if not completly ugly in complex UI... Yes but we also need much better widgets. Because they need to

- accept blocks or provide correct APIs to be able to refresh - for example we got problem because we want to be able to open a button then change its label and the label should reflected.

For me, the first goal of an UIBuilder (graphical or programmatic) is to ease the presentation and to ease the two-way data-binding with domain data/logic. If the ease-of-use of Spec is not more obvious compared to the underlying widget libraries (Morph / Bloc soon maybe...), it will be difficult to convince people to use it in future developments...

Reuse of interaction.

But it is maybe a bit off topic -> "cleaning Spec API".

Let us start by the beginning

Cheers, Glenn.

----- Glenn Cavarlé -- View this message in context: http://forum.world.st/call-for-cleaning-ideas-around-Spec-tp4893494p4893625.... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.

On 05/05/16 00:15, Stephan Eggermont wrote:

On 04/05/16 08:19, stepharo wrote:

...

if you have ideas let us know too.

Why does every NewValueHolder have its own announcer? And is initializing all those unused valueHolders in initialize of Spec presentation classes not slow?

Just a 50721 image I'm working in on SpecGenerator with 20 open windows: NewValueHolder allInstances size 4872 (NewValueHolder allInstances select: [ :each | (each instVarNamed: #announcer) numberOfSubscriptions = 0 ])size 2566

On 05/05/16 00:15, Stephan Eggermont wrote:

On 04/05/16 08:19, stepharo wrote:

...

if you have ideas let us know too.

Why does every NewValueHolder have its own announcer? And is initializing all those unused valueHolders in initialize of Spec presentation classes not slow?

Talking to Latsabben on Slack, first step: Name: SLICE-Issue-18167-NewValueHolder-classvalue-should-use-rawValue-StephanEggermont.1 Author: StephanEggermont Time: 5 May 2016, 11:55:36.083109 am UUID: d89767d0-5d9d-40b8-ad67-5887b22a338f Ancestors: Dependencies: NewValueHolder-StephanEggermont.34 Do not send announcements when you know there can't be any subscriptions

Also maybe let's not push behavior changes two days before release… one of the proposed changes to not fire announcement when the object has changed would actually break things… changes need some time to trigger all the bugs. :) After all the smallest changes usually have the highest bugrate, because when you go fast it's easy to not realize / forget corner cases. Peter On Sun, May 8, 2016 at 10:01 AM, stepharo <stepharo@free.fr> wrote:

Hi stefan

Should we apply that for all the announcements? Do you write tests for this? Stef

Why does every NewValueHolder have its own announcer?

...

...

And is initializing all those unused valueHolders in initialize of Spec presentation classes not slow?

Talking to Latsabben on Slack, first step:

Name: SLICE-Issue-18167-NewValueHolder-classvalue-should-use-rawValue-StephanEggermont.1 Author: StephanEggermont Time: 5 May 2016, 11:55:36.083109 am UUID: d89767d0-5d9d-40b8-ad67-5887b22a338f Ancestors: Dependencies: NewValueHolder-StephanEggermont.34

Do not send announcements when you know there can't be any subscriptions

On 08-05-16 10:01, stepharo wrote:

Should we apply that for all the announcements? Do you write tests for this? We should be very clear about when we're setting things up, and when we expect announcements to start being sent.

The NewValueHolder class method should be safe. Stephan

On 08-05-16 09:59, stepharo wrote:

...

We did an experience by trying to remove the valueHolder is > ButtonPresenter but it led to refresh problem when we want to update > the widget.

If we want to have such fine-grained change propagation with a NewValueHolder for each widgetproperty, we need a much more lightweight announcement proces. NewValueHolder initializes a boolean lock and an Announcer. The announcer initializes a SubscriptionRegistry. The subscriptionregistry initializes a Semaphore and an IdentitySet. An identityset initializes with a tally and a size of 5. If we know most valueholders never need to propagate a change, and most of the others only to one subscriber, we should introduce some null/one objects. Alternatively, we could propagate change at the widget level instead of the property. Stephan

On 08-05-16 17:19, Peter Uhnák wrote:

We should have metrics for this… otherwise seeing what's the actual impact is a guesswork. Most ValueHolders will have at least (and usually) one subscriber — the adapter Most have 0

1741 NewValueHolders, 911 with 0, 801 with 1 NewValueHolder allInstances select: [ :each | (each instVarNamed: #announcer) numberOfSubscriptions = 1 ] Stephan

On 08-05-16 18:34, Peter Uhnák wrote:

If the difference is 10% maybe the word "most" isn't appropriate, but you are looking at system, not Spec.

There is exactly one user of NewValueHolder outside Spec

Most widgets will instantiate the value holder and then immediately subscribe to it (either directly in #initialize, or in #registerEvents), at least for ValueHolders that propagate changes to Morphic. Most properties in a user interface are optional. We need to design for that Regardless of that I said _will_, because that's the idea behind the new Adapters, currently in the image only TabModel/MorphicTabAdapter use it. What is the impact of that on debugability?

Stephan

On Mon, May 9, 2016 at 11:02 AM, Denis Kudriashov <dionisiydk@gmail.com> wrote:

Hi.

I have maybe stupid question. Why Spec needs SpecInterpreter? If I understand correctly it purpose is to parse literal array and create morphs from it. Is it true? If yes then why Spec not uses general literal array parser? (there is one which was born from old discussion about external formats http://smalltalkhub.com/#!/~SvenVanCaekenberghe/ObjectLiterals)

No. Morphs are created by Adapters, Spec interpreter instantiates glues everything together. Regardless, there's no parsing involved… what is there to parse? There are no literal arrays anywhere. The SpecLayout (that we called array last week), is not an array, it's a composition of real objects defaultSpec ^ SpecLayout composed newRow: [ :row | row newColumn: [ :col | col add: #topToolbar height: self toolbarHeight ] ] height: self toolbarHeight; newRow: [ :row | row newColumn: [ :col | col add: #navigatorModel ] origin: 0 @ 0 corner: 0.2 @ 1; addSplitter; newColumn: [ :col | col add: #tabManager ] origin: 0.2 @ 0 corner: 0.85 @ 1; addSplitter; newColumn: [ :col | col add: #formModel ] origin: 0.85 @ 0 corner: 1 @ 1 ] top: self toolbarHeight; yourself Peter

2016-05-04 8:19 GMT+02:00 stepharo <stepharo@free.fr>:

...

Hi guys

this afternoon we will have a meeting with Peter and others to talk about cleaning Spec API.

If you want/can join let us know

else we can organise another skype.

if you have ideas let us know too.

Stef

On Mon, May 9, 2016 at 1:22 PM, Denis Kudriashov <dionisiydk@gmail.com> wrote:

2016-05-09 11:38 GMT+02:00 Peter Uhnák <i.uhnak@gmail.com>:

...

No. Morphs are created by Adapters, Spec interpreter instantiates glues everything together. Regardless, there's no parsing involved… what is there to parse? There are no literal arrays anywhere. The SpecLayout (that we called array last week), is not an array, it's a composition of real objects

defaultSpec ^ SpecLayout composed newRow: [ :row | row newColumn: [ :col | col add: #topToolbar height: self toolbarHeight ] ] height: self toolbarHeight; newRow: [ :row | row newColumn: [ :col | col add: #navigatorModel ] origin: 0 @ 0 corner: 0.2 @ 1; addSplitter; newColumn: [ :col | col add: #tabManager ] origin: 0.2 @ 0 corner: 0.85 @ 1; addSplitter; newColumn: [ :col | col add: #formModel ] origin: 0.85 @ 0 corner: 1 @ 1 ] top: self toolbarHeight; yourself

Maybe array approach deprecated? But in first demos of Spec it was like this:

CheckBoxExample class>>defaultSpec

^ { #ContainerModel. #add:. { self topSpec. #layout:. #(#SpecLayoutFrame bottomFraction: 0 bottomOffset: 20) }. #add:. {{#model . #container } . #layout: . #(#SpecLayoutFrame topOffset: 22) } }

(Actually it is from Pharo 5 image).

Wow, this is ugly. I like the real "self topSpec" call in the middle of it. :) But even then, this is still a regular Pharo (dynamic) array, so no need to parse it… but don't use it like this. :) Peter

Yes, IIRC, the whole idea behind Spec was to have a literal syntax for specifying UI's. (Which is a neat storage format for, say, a UIBuilder) The focus seems to have changed over time to directly using what was initially the intermediate object model the builder generated when parsing the spec. Cheers, Henry

On 09 May 2016, at 1:22 , Denis Kudriashov <dionisiydk@gmail.com> wrote:

2016-05-09 11:38 GMT+02:00 Peter Uhnák <i.uhnak@gmail.com <mailto:i.uhnak@gmail.com>>: No. Morphs are created by Adapters, Spec interpreter instantiates glues everything together. Regardless, there's no parsing involved… what is there to parse? There are no literal arrays anywhere. The SpecLayout (that we called array last week), is not an array, it's a composition of real objects

defaultSpec ^ SpecLayout composed newRow: [ :row | row newColumn: [ :col | col add: #topToolbar height: self toolbarHeight ] ] height: self toolbarHeight; newRow: [ :row | row newColumn: [ :col | col add: #navigatorModel ] origin: 0 @ 0 corner: 0.2 @ 1; addSplitter; newColumn: [ :col | col add: #tabManager ] origin: 0.2 @ 0 corner: 0.85 @ 1; addSplitter; newColumn: [ :col | col add: #formModel ] origin: 0.85 @ 0 corner: 1 @ 1 ] top: self toolbarHeight; yourself

Maybe array approach deprecated? But in first demos of Spec it was like this:

CheckBoxExample class>>defaultSpec

^ { #ContainerModel. #add:. { self topSpec. #layout:. #(#SpecLayoutFrame bottomFraction: 0 bottomOffset: 20) }. #add:. {{#model . #container } . #layout: . #(#SpecLayoutFrame topOffset: 22) } }

(Actually it is from Pharo 5 image).

On 09 May 2016, at 1:53 , Peter Uhnák <i.uhnak@gmail.com> wrote:

(Which is a neat storage format for, say, a UIBuilder)

When you don't touch it, you don't care about it.

The focus seems to have changed over time to directly using what was initially the intermediate object model the builder generated when parsing the spec.

I guess because it's written by hand… I mean the array looks like bad attempt at STON.

In any case, we can always introduce a proper DSL for layout later (or even better, have a UI builder!). Right now it's written with the spec layout.

Peter

Uhuh, in my mind, it seemed to get off to a kind of shaky start, and never improved. Rather than take the opportunity to design a more generic object model for widgets with a clear strategy in mind for mapping to a literal syntax, it seemed to me to have started by copy-pasting the entire API used by Morphic widgets, adding Spec in front of some of the class names, and call it a day. Slap some {}'s around where appropriate, and you have a literal syntax! The good news is, no-one in their right mind would willingly use the literal format as is, so changing it probably wouldn't cause too much of an uproar, the bad new is, all the hard work involved in creating a sensical format, still remains. It's also rather hard to make a good object model that you then later graft a literal format on top of... For instance, with the use of blocks for everything, and the weird mix of symbols and method calls: row newColumn: [ :col | col add: #topToolbar height: self toolbarHeight ] I'd have no idea how to represent as a literal in an intuitive way. Cheers, Henry

On 09 May 2016, at 13:45, Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:

Yes, IIRC, the whole idea behind Spec was to have a literal syntax for specifying UI's. (Which is a neat storage format for, say, a UIBuilder) The focus seems to have changed over time to directly using what was initially the intermediate object model the builder generated when parsing the spec.

btw, I’ve never been happy with that change :) and I agree, the original idea was to have some s-strings based description so an UI builder (to be done) could do a better job. Esteban

Cheers, Henry

...

On 09 May 2016, at 1:22 , Denis Kudriashov <dionisiydk@gmail.com <mailto:dionisiydk@gmail.com>> wrote:

2016-05-09 11:38 GMT+02:00 Peter Uhnák <i.uhnak@gmail.com <mailto:i.uhnak@gmail.com>>: No. Morphs are created by Adapters, Spec interpreter instantiates glues everything together. Regardless, there's no parsing involved… what is there to parse? There are no literal arrays anywhere. The SpecLayout (that we called array last week), is not an array, it's a composition of real objects

defaultSpec ^ SpecLayout composed newRow: [ :row | row newColumn: [ :col | col add: #topToolbar height: self toolbarHeight ] ] height: self toolbarHeight; newRow: [ :row | row newColumn: [ :col | col add: #navigatorModel ] origin: 0 @ 0 corner: 0.2 @ 1; addSplitter; newColumn: [ :col | col add: #tabManager ] origin: 0.2 @ 0 corner: 0.85 @ 1; addSplitter; newColumn: [ :col | col add: #formModel ] origin: 0.85 @ 0 corner: 1 @ 1 ] top: self toolbarHeight; yourself

Maybe array approach deprecated? But in first demos of Spec it was like this:

CheckBoxExample class>>defaultSpec

^ { #ContainerModel. #add:. { self topSpec. #layout:. #(#SpecLayoutFrame bottomFraction: 0 bottomOffset: 20) }. #add:. {{#model . #container } . #layout: . #(#SpecLayoutFrame topOffset: 22) } }

(Actually it is from Pharo 5 image).

Denis this is the same. at the end we get an array and from this array we should get widgets. Stef Le 9/5/16 à 13:22, Denis Kudriashov a écrit :

2016-05-09 11:38 GMT+02:00 Peter Uhnák <i.uhnak@gmail.com <mailto:i.uhnak@gmail.com>>:

No. Morphs are created by Adapters, Spec interpreter instantiates glues everything together. Regardless, there's no parsing involved… what is there to parse? There are no literal arrays anywhere. The SpecLayout (that we called array last week), is not an array, it's a composition of real objects

defaultSpec ^ SpecLayout composed newRow: [ :row | row newColumn: [ :col | col add: #topToolbar height: self toolbarHeight ] ] height: self toolbarHeight; newRow: [ :row | row newColumn: [ :col | col add: #navigatorModel ] origin: 0 @ 0 corner: 0.2 @ 1; addSplitter; newColumn: [ :col | col add: #tabManager ] origin: 0.2 @ 0 corner: 0.85 @ 1; addSplitter; newColumn: [ :col | col add: #formModel ] origin: 0.85 @ 0 corner: 1 @ 1 ] top: self toolbarHeight; yourself

Maybe array approach deprecated? But in first demos of Spec it was like this:

CheckBoxExample class>>defaultSpec

^ { #ContainerModel. #add:.{ self topSpec. #layout:. #(#SpecLayoutFrame bottomFraction: 0 bottomOffset: 20) }. #add:.{{#model . #container } . #layout: . #(#SpecLayoutFrame topOffset: 22) } }

(Actually it is from Pharo 5 image).