Smalltalk › Squeak › Squeak - Dev

Missing Class Method in #MenuIcons

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

5 messages Options

jrm

Missing Class Method in #MenuIcons

The class method #icons is missing from #MenuIcons.

I have attached my tested proposal for a replacement.

- jrm

MenuIcons class-icons.st (324 bytes) Download Attachment

Chris Muller-3

Re: Missing Class Method in #MenuIcons

I think it's wrong for it to be an IdentityDictionary, even though the
keys are Symbols. We should convert it to a plain Dictionary.

On Tue, Apr 24, 2018 at 7:56 PM, John-Reed Maffeo <[hidden email]> wrote:
> The class method #icons is missing from #MenuIcons.
>
> I have attached my tested proposal for a replacement.
>
> - jrm
>
>
>

marcel.taeumel

Re: Missing Class Method in #MenuIcons

Note that there is MenuIcons, ToolIcons, and HelpIcons, which should all work in a similar way.

Best,

Marcel

Am 25.04.2018 03:14:21 schrieb Chris Muller <[hidden email]>:
I think it's wrong for it to be an IdentityDictionary, even though the
keys are Symbols. We should convert it to a plain Dictionary.

On Tue, Apr 24, 2018 at 7:56 PM, John-Reed Maffeo wrote:
> The class method #icons is missing from #MenuIcons.
>
> I have attached my tested proposal for a replacement.
>
> - jrm
>
>
>

jrm

Re: Missing Class Method in #MenuIcons

In reply to this post by Chris Muller-3

I have no opinion, I just copied what was used in #ToolIcons. #HelpIcons currently uses #Dictionary. It makes sense that they would all used the same pattern. I have not made any contributions in quite a while and I am not up to speed with the community development model. I checked the wiki for info about the community development model, but I am not quite sure how to use it. I can access the Repository at http://source.squeak.org/inbox in Monticello Browser, but I am not sure what to do next (or even if I should).

The attached change set updates the class method #icon in #ToolIcons and #MenuIcons to be consistent with the use of #Dictionary in #HelpIcons. All three should now be the same.

My only test was to #do

ToolIcons icons explore.

MenuIcons icons explore.

HelpIcons icons explore.

in a Workspace. Results appeared to be correct.

Cheers,

jrm

On Wed, Apr 25, 2018 at 1:58 PM, Chris Muller <[hidden email]> wrote:

I think it's wrong for it to be an IdentityDictionary, even though the
keys are Symbols. We should convert it to a plain Dictionary.

On Tue, Apr 24, 2018 at 7:56 PM, John-Reed Maffeo <[hidden email]> wrote:
> The class method #icons is missing from #MenuIcons.
>
> I have attached my tested proposal for a replacement.
>
> - jrm
>
>
>

Icons.1.cs (6K) Download Attachment

Chris Muller-4

Re: Missing Class Method in #MenuIcons

Your change is in trunk.

Thank you for the contribution.

On Wed, Apr 25, 2018 at 5:33 PM, John-Reed Maffeo <[hidden email]> wrote:

I have no opinion, I just copied what was used in #ToolIcons. #HelpIcons currently uses #Dictionary. It makes sense that they would all used the same pattern. I have not made any contributions in quite a while and I am not up to speed with the community development model. I checked the wiki for info about the community development model, but I am not quite sure how to use it. I can access the Repository at http://source.squeak.org/inbox in Monticello Browser, but I am not sure what to do next (or even if I should).

The attached change set updates the class method #icon in #ToolIcons and #MenuIcons to be consistent with the use of #Dictionary in #HelpIcons. All three should now be the same.

My only test was to #do

ToolIcons icons explore.
MenuIcons icons explore.
HelpIcons icons explore.

in a Workspace. Results appeared to be correct.
Cheers,
jrm
On Wed, Apr 25, 2018 at 1:58 PM, Chris Muller <[hidden email]> wrote:
I think it's wrong for it to be an IdentityDictionary, even though the
keys are Symbols. We should convert it to a plain Dictionary.

On Tue, Apr 24, 2018 at 7:56 PM, John-Reed Maffeo <[hidden email]> wrote:
> The class method #icons is missing from #MenuIcons.
>
> I have attached my tested proposal for a replacement.
>
> - jrm
>
>
>