System-Announcements: add a ProtocolRenamed announcement

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

System-Announcements: add a ProtocolRenamed announcement

Skip Lentz-2
Hello,

Right now, when you add or remove a protocol to a class you get two announcements (ignoring methods that may have been removed):

1. Protocol{Added,Removed}
2. ClassReorganized.

But when you rename a protocol, you get just ClassReorganized (again, ignoring the method recategorizations).
From this announcement, you only have access to the new class. So there is no way to tell the change.

To resolve this, I think a ProtocolRenamed announcement should be added, just like there is a CategoryRenamed announcement.

This way Epicea can also log this, which it currently does not do. It logs the recategorizations of the methods in the protocol, but not
the rename of the protocol. So as far as we can tell from the log, we can’t see if the renamed protocol still exists or not.

Please share your thoughts,

Skip
Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

tinchodias
Hi,

On Mon, Sep 14, 2015 at 5:02 PM, Skip Lentz <[hidden email]> wrote:
Hello,

Right now, when you add or remove a protocol to a class you get two announcements (ignoring methods that may have been removed):

1. Protocol{Added,Removed}
2. ClassReorganized.

But when you rename a protocol, you get just ClassReorganized (again, ignoring the method recategorizations).
From this announcement, you only have access to the new class. So there is no way to tell the change.

To resolve this, I think a ProtocolRenamed announcement should be added, just like there is a CategoryRenamed announcement.

I think it makes sense, and it is harmless (I mean, shouldn't break anything).

Martín
 

This way Epicea can also log this, which it currently does not do. It logs the recategorizations of the methods in the protocol, but not
the rename of the protocol. So as far as we can tell from the log, we can’t see if the renamed protocol still exists or not.

Please share your thoughts,

Skip

Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

Thierry Goubier
Hi Martin, Skip,

it makes sense, sort of.

ClassReorganized could be handled already by just either discarding whatever cache you would have on the protocols of the class (for example, in a browser) or comparing the new class organisation with the old one.

You may be pointing to, however, another issue: ClassReorganized not carrying enough information (i.e. not the old set of protocols).

I'm not sure your ProtocolRenamed solves that.

But it won't do much harm... Unless Nautilus overeacts and refresh itself 20 times, that it :)

Thierry

2015-09-15 16:26 GMT+02:00 Martin Dias <[hidden email]>:
Hi,

On Mon, Sep 14, 2015 at 5:02 PM, Skip Lentz <[hidden email]> wrote:
Hello,

Right now, when you add or remove a protocol to a class you get two announcements (ignoring methods that may have been removed):

1. Protocol{Added,Removed}
2. ClassReorganized.

But when you rename a protocol, you get just ClassReorganized (again, ignoring the method recategorizations).
From this announcement, you only have access to the new class. So there is no way to tell the change.

To resolve this, I think a ProtocolRenamed announcement should be added, just like there is a CategoryRenamed announcement.

I think it makes sense, and it is harmless (I mean, shouldn't break anything).

Martín
 

This way Epicea can also log this, which it currently does not do. It logs the recategorizations of the methods in the protocol, but not
the rename of the protocol. So as far as we can tell from the log, we can’t see if the renamed protocol still exists or not.

Please share your thoughts,

Skip


Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

Skip Lentz-2

> On Sep 15, 2015, at 4:40 PM, Thierry Goubier <[hidden email]> wrote:
>
> Hi Martin, Skip,
>
> it makes sense, sort of.
>
> ClassReorganized could be handled already by just either discarding whatever cache you would have on the protocols of the class (for example, in a browser) or comparing the new class organisation with the old one.

As far as I know, you can’t compare to the old class organization because it is already changed to the new one..

> You may be pointing to, however, another issue: ClassReorganized not carrying enough information (i.e. not the old set of protocols).

Yes… it only has the class which has been affected. No way to compare it.

> I'm not sure your ProtocolRenamed solves that.

While it may not solve the problem of ClassReorganized lacking information, it does give extra information as to what happened. Right now you simply don’t know when a protocol was renamed.

> But it won't do much harm... Unless Nautilus overeacts and refresh itself 20 times, that it :)

If Nautilus does not subscribe to ProtocolRenamed then nothing happens, no?
I am willing to create an issue, propose a slice, etc.

Thanks for your reply :)

PS: There’s something else related to System-Announcements, namely that when I rename a category I get no CategoryRenamed announcement.
I will send an e-mail about that later.
Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

Thierry Goubier


2015-09-16 10:18 GMT+02:00 Skip Lentz <[hidden email]>:

> On Sep 15, 2015, at 4:40 PM, Thierry Goubier <[hidden email]> wrote:
>
> Hi Martin, Skip,
>
> it makes sense, sort of.
>
> ClassReorganized could be handled already by just either discarding whatever cache you would have on the protocols of the class (for example, in a browser) or comparing the new class organisation with the old one.

As far as I know, you can’t compare to the old class organization because it is already changed to the new one..

Yes but: the receiver of the announcement may have a copy of the old one. For example, when AltBrowser receives that, it clears its current knowledge of the class organisation and rebuilts it.
 

> You may be pointing to, however, another issue: ClassReorganized not carrying enough information (i.e. not the old set of protocols).

Yes… it only has the class which has been affected. No way to compare it.

This is it. But ClassReorganized can be used for reordering the protocols.
 

> I'm not sure your ProtocolRenamed solves that.

While it may not solve the problem of ClassReorganized lacking information, it does give extra information as to what happened. Right now you simply don’t know when a protocol was renamed.

Because, up to know, we managed by either: rebuilding what depends on the organisation, or use it for something else.

For example, usually in Smalltalk, protocols are ordered and a user may reorder them by hand -> ClassReorganized. Nautilus takes a different view (order alphabetically, add a all protocol), hence Pharo protocols have integrated the Nautilus viewpoint (the 'all' protocol), and AltBrowser disregard this (the 'all' protocol and the ordering) because I prefer a different way of ordering.
 

> But it won't do much harm... Unless Nautilus overeacts and refresh itself 20 times, that it :)

If Nautilus does not subscribe to ProtocolRenamed then nothing happens, no?
I am willing to create an issue, propose a slice, etc.

One more thing. I hope that when renaming the protocol, the relevant MethodRecategorized are sent, because many things depend on that one.

I need to check AltBrowser behavior on this one. Renaming a protocol is not what I expected. It looks harmless, but it has important consequences if the protocol is an extension or an override.
 

Thanks for your reply :)

PS: There’s something else related to System-Announcements, namely that when I rename a category I get no CategoryRenamed announcement.
I will send an e-mail about that later.

Hum, SystemOrganizer>>#renameCategory:toBe: sends it (in Pharo4).

Thierry

Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

Skip Lentz-2

On Sep 16, 2015, at 10:36 AM, Thierry Goubier <[hidden email]> wrote:



2015-09-16 10:18 GMT+02:00 Skip Lentz <[hidden email]>:

As far as I know, you can’t compare to the old class organization because it is already changed to the new one..

Yes but: the receiver of the announcement may have a copy of the old one. For example, when AltBrowser receives that, it clears its current knowledge of the class organisation and rebuilts it.

Ah yes, that way it is possible.

 

> I'm not sure your ProtocolRenamed solves that.

While it may not solve the problem of ClassReorganized lacking information, it does give extra information as to what happened. Right now you simply don’t know when a protocol was renamed.

Because, up to know, we managed by either: rebuilding what depends on the organisation, or use it for something else.

For example, usually in Smalltalk, protocols are ordered and a user may reorder them by hand -> ClassReorganized. Nautilus takes a different view (order alphabetically, add a all protocol), hence Pharo protocols have integrated the Nautilus viewpoint (the 'all' protocol), and AltBrowser disregard this (the 'all' protocol and the ordering) because I prefer a different way of ordering.
 

> But it won't do much harm... Unless Nautilus overeacts and refresh itself 20 times, that it :)

If Nautilus does not subscribe to ProtocolRenamed then nothing happens, no?
I am willing to create an issue, propose a slice, etc.

One more thing. I hope that when renaming the protocol, the relevant MethodRecategorized are sent, because many things depend on that one.

Yes, those are already sent. ProtocolRenamed, if implemented, should be announced in addition to those.

I need to check AltBrowser behavior on this one. Renaming a protocol is not what I expected. It looks harmless, but it has important consequences if the protocol is an extension or an override.

Alright, let me know.

 

Thanks for your reply :)

PS: There’s something else related to System-Announcements, namely that when I rename a category I get no CategoryRenamed announcement.
I will send an e-mail about that later.

Hum, SystemOrganizer>>#renameCategory:toBe: sends it (in Pharo4).

Indeed it does, but RPackageTag>>#renameTo:category: suspends the SystemAnnouncer while it sends SystemOrganizer>>#renameCategory:toBe:.
I don’t know why, I’m interested to know. Because RPackageTag>>#renameTo:category: is used when you rename a category (in Nautilus, at least).

Skip
Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

Thierry Goubier


2015-09-16 11:05 GMT+02:00 Skip Lentz <[hidden email]>:
 
... 

One more thing. I hope that when renaming the protocol, the relevant MethodRecategorized are sent, because many things depend on that one.

Yes, those are already sent. ProtocolRenamed, if implemented, should be announced in addition to those.

I need to check AltBrowser behavior on this one. Renaming a protocol is not what I expected. It looks harmless, but it has important consequences if the protocol is an extension or an override.

Alright, let me know.

I think you can go forward. If the MethodRecategorized is sent, then it should be OK.
 

 

Thanks for your reply :)

PS: There’s something else related to System-Announcements, namely that when I rename a category I get no CategoryRenamed announcement.
I will send an e-mail about that later.

Hum, SystemOrganizer>>#renameCategory:toBe: sends it (in Pharo4).

Indeed it does, but RPackageTag>>#renameTo:category: suspends the SystemAnnouncer while it sends SystemOrganizer>>#renameCategory:toBe:.
I don’t know why, I’m interested to know. Because RPackageTag>>#renameTo:category: is used when you rename a category (in Nautilus, at least).

I would heed Esteban advice on that. If the system announcer is suspended, then this is probably because we may have some kind of infinite loop and / or a incoherent state while renaming the system category.

I'll have a look too.

Thierry
 

Skip

Reply | Threaded
Open this post in threaded view
|

Re: System-Announcements: add a ProtocolRenamed announcement

stepharo
Thanks thierry to look and discuss these points. I do not have enough calm time to get any concentration (of course I was teaching
all day long).

Stef

Le 16/9/15 09:20, Thierry Goubier a écrit :


2015-09-16 11:05 GMT+02:00 Skip Lentz <[hidden email]>:
 
... 

One more thing. I hope that when renaming the protocol, the relevant MethodRecategorized are sent, because many things depend on that one.

Yes, those are already sent. ProtocolRenamed, if implemented, should be announced in addition to those.

I need to check AltBrowser behavior on this one. Renaming a protocol is not what I expected. It looks harmless, but it has important consequences if the protocol is an extension or an override.

Alright, let me know.

I think you can go forward. If the MethodRecategorized is sent, then it should be OK.
 

 

Thanks for your reply :)

PS: There’s something else related to System-Announcements, namely that when I rename a category I get no CategoryRenamed announcement.
I will send an e-mail about that later.

Hum, SystemOrganizer>>#renameCategory:toBe: sends it (in Pharo4).

Indeed it does, but RPackageTag>>#renameTo:category: suspends the SystemAnnouncer while it sends SystemOrganizer>>#renameCategory:toBe:.
I don’t know why, I’m interested to know. Because RPackageTag>>#renameTo:category: is used when you rename a category (in Nautilus, at least).

I would heed Esteban advice on that. If the system announcer is suspended, then this is probably because we may have some kind of infinite loop and / or a incoherent state while renaming the system category.

I'll have a look too.

Thierry
 

Skip