Introduce SpMenuPresenter>>#fillWith: and add tests and example

[koendehondt]

Code changes for https://github.com/pharo-spec/Spec/issues/1601.

• Addition of SpCommandGroup>>#asToolbarPresenter
• Addition of SpCommandGroup>>#asToolbarPresenterWith:
• Addition of example
• Some code cleanup and class comment improvements

[koendehondt]

This change is important for the Spec book. Please merge the PR to Pharo 12 as well.

[estebanlm]

thing is, I removed this precisely for a reason: it does not produces a correct "DOM", and I cannot be sure that the resulting toolbar will be attached to a parent or an application (hence, I cannot know if backend will be right) :) In this case, I dropped this in favor of the fillWith: idiom, which will end working like this:

toolbar := self newToolbar.
toolbar fillWith: aCommandGroup
... etc...

and with this approach, I do not need two methods, since the configuration of the toolbar can happen where it is created.

[estebanlm]

uhm... but now I see that there are other idioms (asMenuPresenter, asMenuBarPresenter) in the same problem, I need to fix those too...

[koendehondt]

uhm... but now I see that there are other idioms (asMenuPresenter, asMenuBarPresenter) in the same problem, I need to fix those too...

Please do, so that we do not write wrong patterns in the Spec book. cc @Ducasse

[Ducasse]

Yes this is really painful to write a book more than people can think.

[koendehondt]

@estebanlm

thing is, I removed this precisely for a reason: it does not produces a correct "DOM", and I cannot be sure that the resulting toolbar will be attached to a parent or an application (hence, I cannot know if backend will be right) :) In this case, I dropped this in favor of the fillWith: idiom

I read the code and tried some examples. You are right. The fillWith: idom is the way to go. Sadly, there are 33 senders of SpCommandGroup>>#asMenuPresenter, which give bad examples to developers.

uhm... but now I see that there are other idioms (asMenuPresenter, asMenuBarPresenter) in the same problem, I need to fix those too...

Please read the changes in this PR. I adapted the code:

• Remove SpCommandGroup>>#asToolbarPresenter and SpCommandGroup>>#asToolbarPresenterWith: that I added initially.
• Adapted the example SpCommandGroupExample to use SpToolbarPresenter>>#fillWith:.
• Added SpMenuPresenter>>#fillWith: and related methods. Note the move of the initialization of the stack from SpMenuPresenterBuilder>>#initialize to SpMenuPresenterBuilder>>#menuPresenter:, otherwise building a menu or menubar would not work with a given menu or menubar (stack would still hold the default menuPresenter).
• Added tests for the two new methods.
• Adapted the example to send fillWith: instead of asMenuBarPresenter.
• Added a context menu to the example, filling it with fillWith:.

I will rename this PR to express the changes.

I did not:

• Remove SpCommandGroup>>#asMenuPresenter, as it has many senders.
• Remove SpCommandGroup>>#asMenuPresenterWith:, although it has no senders.
• Remove SpCommandGroup>>#asMenuBarPresenter, although it has no senders.
• Remove SpCommandGroup>>#asMenuBarPresenterWith:, although it has no senders.

The removal should be part of another Github ticket to clean up the senders. That is out of the scope of this ticket.

If you approve the changes, I will also be happy to make a PR for Pharo 12, as Pharo 12 is the stable version the Spec book is based on.

[koendehondt]

I forgot to mention: I fixed a bug in SpMenuBarPresenterTest. SpMenuBarPresenterTest>>#classToTest was missing, so that the test class used SpMenuPresenter as the class to test, not SpMenuBarPresenter.