[Pharo-project] Issue 3815 in pharo: Filing in Class comments raises an error
Status: Accepted Owner: ryd...@gmail.com Labels: Milestone-1.2 Milestone-1.3 New issue 3815 by ryd...@gmail.com: 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 ryd...@gmail.com: 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 stephane...@gmail.com: 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 ryd...@gmail.com: 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 stephane...@gmail.com: 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 stephane...@gmail.com: 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 stephane...@gmail.com: 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 stephane...@gmail.com: 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 ryd...@gmail.com: 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 stephane...@gmail.com: 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 marcus.d...@gmail.com: 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 stephane...@gmail.com: 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 ryd...@gmail.com: 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 stephane...@gmail.com: 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 ryd...@gmail.com: 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.
participants (1)
-
pharo＠googlecode.com