VisualGST UI

HI,

So here is the big monster, I've migrated most of the UI to a new UI
framework
less Gtk dependant. You can see that mainWidget is not called anymore in
the
browser, launcher, debugger, ... Another improvement is done in the widget
initialization the initialize method is called for all the subclasses of
Widget.
Most of the widgets are migrated to the new model and new widgets.

Off course this is only the first step of the migration there are still
some works
(more widgets, remove GTK.*, improve events, ...) but that's a good step.

Gwen


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Wed, Oct 23, 2013 at 12:24:32PM +0200, Gwenaël Casaccio wrote:
> HI,

Good Morning,

>
> So here is the big monster,

hehe, I really prefer evolution and small steps (as they can be
reviewed and undone more easily)

>      debuggerClass [
>          <category: '*VisualGST-debugger'>
>  
> -        ^ VisualGST.GtkDebugger
> +        ^ nil
> +"        ^ VisualGST.GtkDebugger"
>      ]


left over. :)

> +        | iter |
> +        iter := self findIter: anObject ifAbsent: [ ^ aBlock value ].
> +        model remove: iter
> +    ]
> +
> +    remove: anObject [
> + <category: 'model'>
> +
> + self remove: anObject ifAbsent: [ self error: 'item not found' ]

you appear to mix tabs and spaces. Please pick one of the two.


> - GtkAnnouncer current on: GtkNamespaceSelectionChanged do: [ :ann |
> + "GtkAnnouncer current on: GtkNamespaceSelectionChanged do: [ :ann |
>      browsers updateWidget: browsers currentWidget withLabel: ann selectedNamespace name asString].
>   GtkAnnouncer current on: GtkClassSelectionChanged do: [ :ann |
> -    browsers updateWidget: browsers currentWidget withLabel: ann selectedClass printString]
> +    browsers updateWidget: browsers currentWidget withLabel: ann selectedClass printString]"
>      ]

left over?



This patch is impossible to review but the client code appears to get
more simple indeed. I wish you could break this into smaller logical
pieces.

thanks
        holger

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On 24/10/2013 07:55, Holger Hans Peter Freyther wrote:
> On Wed, Oct 23, 2013 at 12:24:32PM +0200, Gwenaël Casaccio wrote:
>> HI,
> Good Morning,
>
>> So here is the big monster,
> hehe, I really prefer evolution and small steps (as they can be
> reviewed and undone more easily)

I agree but the changes here are too deep to be done in multiples small
steps

For debugging purpose ;D

Yeah maybe I've used another text editor

>> - GtkAnnouncer current on: GtkNamespaceSelectionChanged do: [ :ann |
>> + "GtkAnnouncer current on: GtkNamespaceSelectionChanged do: [ :ann |
>>      browsers updateWidget: browsers currentWidget withLabel: ann selectedNamespace name asString].
>>   GtkAnnouncer current on: GtkClassSelectionChanged do: [ :ann |
>> -    browsers updateWidget: browsers currentWidget withLabel: ann selectedClass printString]
>> +    browsers updateWidget: browsers currentWidget withLabel: ann selectedClass printString]"
>>       ]
> left over?

Temporary disabled for testing SUnit package loading

>
>
>
> This patch is impossible to review but the client code appears to get
> more simple indeed. I wish you could break this into smaller logical
> pieces.
>
> thanks
> holger


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Thu, Oct 24, 2013 at 08:52:31AM +0200, Gwenaël Casaccio wrote:

> I agree but the changes here are too deep to be done in multiples
> small steps

that is the typical GNOME developer statement and I seldomly see it
to be true. With "CADT model" one needs to be very careful to not
confuse change with progress. ;)

>
> Temporary disabled for testing SUnit package loading

What does it mean? SUnit is currently broken? Or it would be broken
with this enabled?

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On 24/10/2013 09:55, Holger Hans Peter Freyther wrote:
> On Thu, Oct 24, 2013 at 08:52:31AM +0200, Gwenaël Casaccio wrote:
>
>> I agree but the changes here are too deep to be done in multiples
>> small steps
> that is the typical GNOME developer statement and I seldomly see it
> to be true. With "CADT model" one needs to be very careful to not
> confuse change with progress. ;)

Well I'm replying for myself I agree but I'm quite happy with my refactoring
all the tools (Inspector, Browser, Debugger, SUnit) are still working
with the patches
I've sent.

>
>> Temporary disabled for testing SUnit package loading
> What does it mean? SUnit is currently broken? Or it would be broken
> with this enabled?

It would be broken with this enabled I need to update the Browser widget
to support the events.


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Thu, Oct 24, 2013 at 10:04:10AM +0200, Gwenaël Casaccio wrote:

> Well I'm replying for myself I agree but I'm quite happy with my refactoring
> all the tools (Inspector, Browser, Debugger, SUnit) are still
> working with the patches
> I've sent.

Please don't get me wrong I see the value of these changes (e.g. my
remark that client code seems to change in a positive way). I have just
heard these[1] excuses for too many times.

For these changes I accept that this is a radical change and can not
be reviewed. But please make previous patches stable first. I will then
merge the UI changes without further comments. Deal?



[1]
 * It can't be tested
 * It is a complete re-write
 * It can't be made smaller

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On 24/10/2013 10:14, Holger Hans Peter Freyther wrote:
Yes sorry I didn't want to be rude. I think the main mistake is much
more the lack
of free time. I agree if I can allow all my time to VisualGST the patch
would be totally
different (first I could draw a raodmap, do incremental changes, ...)
unfortunately it's
not the case and I've to deal with it. On the other hand I would like to
take the opportunity
to change in a good way the UI code which wasn't good as I want.

Gwen


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Wed, Oct 23, 2013 at 12:24:32PM +0200, Gwenaël Casaccio wrote:

Can you please rebase. git am -3 fails as it doesn't know the parent
commits your patch refers to.



_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk