Issue 3815 in pharo: Filing in Class comments raises an error

Status: Accepted
Owner: [hidden email]
Labels: Milestone-1.2 Milestone-1.3

New issue 3815 by [hidden email]: Filing in Class comments raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

Broken by SourceReference changes.

ChangeRecord >> fileIn directly notifies deprecated RecentMessageSet.

It seems to me like the #comment:stamp: call should correctly update  
RecentMessageList through the SystemChangeNotifier, and the calls could  
simply be removed.
Need external confirmation of this though, as I have not beeen involved,  
and have no idea what the implications/consequences of RecentMessageSet  
deprecation really is.

PS. We need tests for fileIns of different kinds, all tests passing for 1.2  
without catching stuff like this is bad :)

Comment #1 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

And if it is truly deprecated, and not planned to be updated to work  
correctly, "Recent submissions" should be removed from the World Tools  
menu...

Comment #2 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

ok push your cs somewhere so that we can have a look

Comment #3 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

Use Tools-> File Browser to open phase1.1.cs from Issue 3814, browse the  
changes, and select the two second and third from the top.
(Class creation doit  and class comment for the same class).
It will give the error when you click "File in selections"

Updates:
        Labels: -Milestone-1.2

Comment #4 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

Using the FileList I can on phase1.1
Ok I see

- changelist browser
- then select class comment
- then file in selection

You can get the complete file loaded using filein btw
And the underlying code is terrible.
So fixing it is a hell.

Comment #5 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

Yes this code is crap. It uses a compiled method as a class comment.... Do  
you imagine
selector #Comment because you have a comment in the cs.
So easy you take a compiled method and bend it so much to the point where  
it looks like a code comment.

Comment #6 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

A dirty solution is to remove the class comments change notification from  
the fileIn.

fileIn
        "File the receiver in.  If I represent a method or a class-comment, file  
the method in and make a note of it in the recent-submissions list; if I  
represent a do-it, then, well, do it."

        Cursor read
                showWhile: [
                        | methodClass aSelector s |
                        (methodClass := self methodClass) notNil
                                ifTrue: [
                                        methodClass
                                                compile: self text
                                                classified: category
                                                withStamp: stamp
                                                notifying: nil.
                                        (aSelector := self methodSelector)
                                                ifNotNil: [ RecentMessageSet noteMethodSubmission: aSelector  
forClass: methodClass ] ].
                        type == #doIt
                                ifTrue: [
                                        ((s := self string) beginsWith: '----')
                                                ifFalse: [ Compiler evaluate: s ] ].
                        ]

Comment #7 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

No recent submission is working. Now we should fix all the metamodel  
underneath and this is not something that can easily be done. We are  
working on the ring metamodel to be able to do that in the future.
So the UI was redbuilt by benjamin and the source ugly model should be  
incrementally migrated to use ring API
and Ring objects.

Updates:
        Labels: Milestone-1.2

Comment #8 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

Never the less, not being able to selectively install changes from a change  
set is clearly a regression from 1.1.1, and should not be pushed to 1.3

Updates:
        Status: Closed

Comment #9 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

in 13086

Comment #10 on issue 3815 by [hidden email]: Filing in Class  
comments raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

should this be added to 1.2, too?

Updates:
        Labels: -Milestone-1.2

Comment #11 on issue 3815 by [hidden email]: Filing in Class  
comments raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

(No comment was entered for this change.)

Comment #12 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

IMHO, yes.
Stef is much closer and scarier, and do not seem to think so though :)

Comment #13 on issue 3815 by [hidden email]: Filing in Class  
comments raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

Henrik I think that this is clearly a showstopper for 1.2 and this will be  
just a patch on a huge patch.
I prefer to keep my energy to rebuild something from scratch using ring

Comment #14 on issue 3815 by [hidden email]: Filing in Class comments  
raises an error
http://code.google.com/p/pharo/issues/detail?id=3815

So, it's ok to leave stuff we broke between versions broken if we intend to  
replace it with something better in some future version?

I don't see the logic in that.