Hi!
Related with the issue http://code.google.com/p/pharo/issues/detail?id=2757 (Issue 2757: Refactor StringHolder) I was looking at the StringHolder hierarchy, and well, it's not very nice. Playing to see how difficult should it be to refactor the hierarchy and decouple a bit those things, I took Workspace off from there, and writing 2 methods it happened to work well (at least with what I tested, hehe). Sooo, I wondered why a StringHolder, what responsibilities it has, and what ones should it have... I found that most of the messages a StringHolder understands are for browsing things and inspect things, such as the Paragraph editor does :). And that's why Workspace could do all his stuff when I made him inherit from Object... StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method." And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :). I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P. Well, this mail is just for ask for advice and argue about the correctness of this change (or not). Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Benjamin started and rewrote the recentMessages and friends. If people can review his code it would be great. > Related with the issue http://code.google.com/p/pharo/issues/detail?id=2757 (Issue 2757: Refactor StringHolder) I was looking at the StringHolder hierarchy, and well, it's not very nice. Playing to see how difficult should it be to refactor the hierarchy and decouple a bit those things, I took Workspace off from there, and writing 2 methods it happened to work well (at least with what I tested, hehe). > > Sooo, I wondered why a StringHolder, what responsibilities it has, and what ones should it have... I found that most of the messages a StringHolder understands are for browsing things and inspect things, such as the Paragraph editor does :). And that's why Workspace could do all his stuff when I made him inherit from Object... I think that we should - Replace ParagraphEditor by SmalltalkEditor, TextEditor (did not check but it should be like that in CUIS and Squeak) In Squeak TextMorph >> initialize "TextMorph initialize" "Initialize the default text editor class to use" DefaultEditorClass := SmalltalkEditor. - then we should probably rewrite some parts and do not inherit from StringHolder I never understood why MessageSet is a subclass of Browser :) But I would focus on the > StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method." > > And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :). This is exact. > I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P. So could you review benjamin code? then check the texteditor/paragraphEditor > > Well, this mail is just for ask for advice and argue about the correctness of this change (or not). > > Thanks for reading up to here! > Guille > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
On Tue, Aug 10, 2010 at 6:00 AM, Stéphane Ducasse <[hidden email]> wrote:
Ok, I'll watch it a few minutes this evening.
What's CUIS? :(
on the ...?
This evening I'll check it and compare it with the existant RecentMessageSet and that stuff. then check the texteditor/paragraphEditor I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it?
_______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Hi Guille,
CUIS is Juan Vuletich's Smalltalk-80 environment derived from Squeak. See http://www.jvuletich.org/Cuis/Index.html Francisco 2010/8/10 Guillermo Polito <[hidden email]>: > > > On Tue, Aug 10, 2010 at 6:00 AM, Stéphane Ducasse > <[hidden email]> wrote: >> >> >> Benjamin started and rewrote the recentMessages and friends. >> If people can review his code it would be great. > > Ok, I'll watch it a few minutes this evening. > >> >> > Related with the issue >> > http://code.google.com/p/pharo/issues/detail?id=2757 (Issue 2757: Refactor >> > StringHolder) I was looking at the StringHolder hierarchy, and well, it's >> > not very nice. Playing to see how difficult should it be to refactor the >> > hierarchy and decouple a bit those things, I took Workspace off from there, >> > and writing 2 methods it happened to work well (at least with what I tested, >> > hehe). >> > >> > Sooo, I wondered why a StringHolder, what responsibilities it has, and >> > what ones should it have... I found that most of the messages a >> > StringHolder understands are for browsing things and inspect things, such as >> > the Paragraph editor does :). And that's why Workspace could do all his >> > stuff when I made him inherit from Object... >> >> I think that we should >> - Replace ParagraphEditor by SmalltalkEditor, TextEditor >> (did not check but it should be like that in CUIS and >> Squeak) > > What's CUIS? :( > >> >> In Squeak >> >> TextMorph >> initialize >> "TextMorph initialize" >> "Initialize the default text editor class to use" >> DefaultEditorClass := SmalltalkEditor. >> >> >> - then we should probably rewrite some parts and do not inherit >> from StringHolder >> I never understood why MessageSet is a subclass of Browser :) >> But I would focus on the >> > > on the ...? > >> >> > StringHolder class comment says: "I am a kind of Model that includes a >> > piece of text. In some cases, the text can be edited, and in some the text >> > is a method." >> > >> > And I smell that its main responsibility is to hold a Text or a String >> > in its contents variable, and all its subclasses are there because of >> > that... And if so, I don't like that inheritance approach :). >> >> This is exact. >> >> > I want to destroy (literally) that hierarchy :). And maybe it helps us >> > in coming refactors and cleanings, so we can do them with less fear :P. >> >> So could you >> review benjamin code? > > This evening I'll check it and compare it with the existant RecentMessageSet > and that stuff. > >> >> then check the texteditor/paragraphEditor > > I'll write it down in my TODO list :). But the idea is to replace > ParagraphEditor with them, is it? > >> >> > >> > Well, this mail is just for ask for advice and argue about the >> > correctness of this change (or not). >> > >> > Thanks for reading up to here! >> > Guille >> > _______________________________________________ >> > Pharo-project mailing list >> > [hidden email] >> > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project >> >> >> _______________________________________________ >> Pharo-project mailing list >> [hidden email] >> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
In reply to this post by Guillermo Polito
>
> What's CUIS? :( another squeak fork > - then we should probably rewrite some parts and do not inherit from StringHolder > I never understood why MessageSet is a subclass of Browser :) > But I would focus on the > > > on the ...? paragraphEditor kicking out > > > > StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method." > > > > And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :). > > This is exact. > > > I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P. > > So could you > review benjamin code? > > This evening I'll check it and compare it with the existant RecentMessageSet and that stuff. > > then check the texteditor/paragraphEditor > > I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it? yes > > > > > Well, this mail is just for ask for advice and argue about the correctness of this change (or not). > > > > Thanks for reading up to here! > > Guille > > _______________________________________________ > > Pharo-project mailing list > > [hidden email] > > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Ok, I checked CUIS and it is using SmalltalkEditor and TextEditor as expected, and there is no notice of ParagraphEditor :).
In squeak 4.1, there are yet some usages of ParagraphEditor, but in some View objects I don't see in Pharo nor in CUIS. I opened an issue for replacing ParagraphEditor: http://code.google.com/p/pharo/issues/detail?id=2778 Cheers!
On Tue, Aug 10, 2010 at 10:12 AM, Stéphane Ducasse <[hidden email]> wrote: > _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
great!
Stef On Aug 10, 2010, at 8:19 PM, Guillermo Polito wrote: > Ok, I checked CUIS and it is using SmalltalkEditor and TextEditor as expected, and there is no notice of ParagraphEditor :). > In squeak 4.1, there are yet some usages of ParagraphEditor, but in some View objects I don't see in Pharo nor in CUIS. > I opened an issue for replacing ParagraphEditor: > > http://code.google.com/p/pharo/issues/detail?id=2778 > > Cheers! > > On Tue, Aug 10, 2010 at 10:12 AM, Stéphane Ducasse <[hidden email]> wrote: > > > > What's CUIS? :( > > another squeak fork > > > - then we should probably rewrite some parts and do not inherit from StringHolder > > I never understood why MessageSet is a subclass of Browser :) > > But I would focus on the > > > > > > on the ...? > > paragraphEditor kicking out > > > > > > > > StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method." > > > > > > And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :). > > > > This is exact. > > > > > I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P. > > > > So could you > > review benjamin code? > > > > This evening I'll check it and compare it with the existant RecentMessageSet and that stuff. > > > > then check the texteditor/paragraphEditor > > > > I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it? > > yes > > > > > > > > > Well, this mail is just for ask for advice and argue about the correctness of this change (or not). > > > > > > Thanks for reading up to here! > > > Guille > > > _______________________________________________ > > > Pharo-project mailing list > > > [hidden email] > > > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > > > > _______________________________________________ > > Pharo-project mailing list > > [hidden email] > > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > > _______________________________________________ > > Pharo-project mailing list > > [hidden email] > > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > _______________________________________________ > Pharo-project mailing list > [hidden email] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project _______________________________________________ Pharo-project mailing list [hidden email] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project |
Free forum by Nabble | Edit this page |