[MonoDevelop] Larger set of stetic patches

ml ml at arsis.net
Fri Jun 30 18:11:17 EDT 2006


First I have to say "I'm sorry" to Lluis, because I already spammed him
with part of this answer directly accidentally bypassing the list

On Thu, 2006-06-29 at 17:57 +0200, Lluis Sanchez wrote:
> Ok, so right now Stetic gets the information about widgets from an
> objects.xml file embedded in the library. What you are proposing is to
> read that information, or at least part of it, from the widget classes
> using reflection. That's an important change in the model, so it needs
> some thinking.
> 

Agreed. I will explain later why my patch doesn't break anything and
what you all missed.

> There are at least two scenarios in which an xml description is
> necessary and can't be replaced by reflection:
> 
>       * When we can't modify the widget library to add attributes.
>         That's the case of gtk#.
>       * When building a dll which implements both widgets and windows
>         that contain instances of those widgets. The problem in this
>         case is that we need to have the widget description before
>         compiling, but we don't have a dll to reflect.
> 

yep, and in the same time you're imposing this structure of yours on
those who can't link on either stetic or monodevelop.

My example is a clear cut case. Can't link against either monodevelop or
stetic (linking against stetic would be pure consumption without one
single gain in my case and I'm already cutting the red line, linking
against monodevelop is impossible not only for the same reason as
stetic, but also unclean license it comes with, the last I followed you
still haven't finished all the #develop/monodevelop license war)

> So we still need to support the xml model, and I'm not sure it is a good
> idea to provide another model for describing widgets. In any case I
> don't think that mixing both models is good. A library should describe a
> widget either using xml or reflection, but not a mix of both.
> 

Mixing? That wouldn't be good, I agree. What you missed in my patch it
is the fact that it isn't mixing, it is priority overriding.

When one mixes two approaches strange things can happen'
When one uses priority override it is for various reasons.
- To retain all previous functionality while safely move on the new one
- To be able to gradually move to new approach but at the same time not
breaking anything
- To be able to safely add functionality the new way and then after that
focus on the old functional part that already works

> > 
> > These patches include: 
> > 
> > 1. ToolboxItem kind of specifying of widgets (but you still have to
> > specify export widget in project options)
> > 
> > [ToolboxItem (true)]
> > [Category ("widgetgroup")]
> > [Description ("Description as posed as label to icon"]
> > public class SomeWidget
> > {
> > ...
> > }
> 
> If widgets still have to be specified in the export widget option panel,
> what's the point of using [ToolboxItem]?
> 

One simple answer. Monodevelop should automatically add
ToolboxItem(true) to exported widgets.

I plan to add this some time later, but for now I have to focus on
different topics.

More complex answer, because approach with specifying exported widgets
and objects.xml is much dirtier to maintain than with attributes?

> > 
> > 
> > 
> > 2. Adding new groups to stetic from code on loading library
> > (ToolboxItem(false) and abstract are conditions here). Original .Net
> > does not provide groups, stetic requires them
> > 
> > [ToolboxItem (false), Category ("widgetgroup"), Description ("Group
> > Label"]
> > public abstract class SomeClass { /* dummy abstract class */ }
> 
> This is really ugly. If a widget library needs to define new categories,
> it should do it in the objects.xml file.
> 

Agreed and thrown out. M.J.'s suggestion about category was really good
and this new patch adopts his way.

> > 
> > 
> > 
> > 3. Correction to objects.xml for SpinButton, second itemgroup "Spin
> > Button Properties" was not inheritable because it didn't provide name
> 
> Ok.
> 

This patch still contains solution for this. My current patch is done
according to http://svn.myrealbox.com/viewcvs/trunk/stetic/ which is 11
days old and doesn't contain it?

> > 
> > 
> > 
> > 4. Real loading when specifying -lib:<somelibrary>, previous code was
> > missing actual loading
> 
> Ok.
> 

Again the cvs question

> > 
> > 
> > 
> > 5. Added possibility for control to specify CreateWrappedInstance
> > method, without this wrapped controls couldn't work without being
> > referenced to stetic. This enables stetic to use base wrapper and use
> > re
> > classed widget. It basically enables for widgets to be fully
> > functional
> > without being referenced to stetic.
> 
> Can you give an example in which is this needed?
> 

As I already said, for various reasons I can't reference against
stetic/monodevelop.

Now, lets look at what monodevelop provides so far (don't get me wrong
on this one, I might be bitchin' about stetic and monodevelop, but they
are still tools of my preference, stetic because it is probably THE most
wonderful form editor (but only as far as editing form and properties,
functionally it has a long way to go) I've used so far and monodevelop
because of its way it does/enables things to happen'. And it is my
belief that saying something is good is not even nearly as good as
saying where that great thing is bad).

- One can do control like you provided: by specifying "New Widget" and
designing that control in stetic and source. All good and very nice. But
this is only usable when one does complex controls, and completely and
utterly stupid for sub classing them. It would mean you would have to
provide every signal and every property to the BIN container stetic
relates too. Now take a look on any site where free controls for any
language are provided, what do you see more of subclassed or complex.
Complex would account for less than 1%. Meaning New widget is functional
but practically unused in real world. Now try specifying "New widget"
and subclassing some widget type other than bin.

- stetic in monodevelop so far can't create widgets that are not loaded
in. Basically, not-even one subclassed gtk control can't be used inside,
unless monodevelop would load this library initially. Understandable
problem, this is one of the .Net stupidities, how .Net works with loaded
assemblies, you can't unload assembly, you can only unload AppDomain.
And as far as I made a quick look in monodevelop insides you're using
Cecil to avoid this fact. Well, as much as Cecil is great for coding, it
is probably not so great on using them.

- another fact is that stetic makes its own way of doing, which makes it
completely impossible to subclass for example simple RadioButton without
referencing to libstetic. It is the way how wrappers work in stetic. If
wrapper is not specified, stetic looks trough base classes where this
custom control was derived from. If one of the base classes specified
wrapper? It creates exactly that one, which creates base controls type
widget instead of this controls type. Which result in stetic
Exceptioning out on accessing properties that were not present in the
base class. And being able to add properties is one of the common basic
functionalities when one is subclassing smoe control type. So,... what
does this patch provides? For controls functionality, nothing, but for
wrappers, a lot. Wrapper can use right type of widget this way, but at
the same time one doesn't need to link against stetic/monodevelop.

Ok, a lot of bitchin' from me:) but what am I doing with this? My plan
is to make DBusified stetic (don't worry, it is a separate app derived
from base stetic, which would need only one functionality to enable this
possibility... (in my case, steticremote) ability to connect to Project
signals, which would be more than adequate to make DBusified stetic, but
at the same time not changing a single bit (except adding few lines
where project is passing events to connected parties) of this one or the
one in monodevelop) instead of hardcore one in monodevelop. This way
stetic could load/reload/unload without bothering monodevelop (or any
other controlling software), basically my way would enable in
monodevelop (or in my case my own controlling software) to say "stetic
load thislib, thatlib and thatproject", all would load and no
constrictions would be posed. And since it would be its own separate
entity, a lot more functionality (which now can't be) could be added to
it. Remote stetic would mean that monodevelop wouldn't bog down stetic
and stetic wouldn't bog down monodevelop. I'm after the "live"
functionality here. Currently as it is stetic is more or less "dead"
functional, anyone saying differently probably hasn't seen functional
form editor in his life.

btw. My patch already provides/solves everything specified here, ok,
everything but ability to connect to Project signals. I plan to this
patch later today

> More comments inline...
> 

ok, comments from me inside too

> > 
> > 
> > 
> > m.
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > fitxer adjunt
> > differences
> > between files
> > (toolboxitem_groups_few_errors.patch)
> > 
> 
> > (snip)
> 
> > stetic.clean/libstetic/ClassDescriptor.cs
> > --- stetic/libstetic/ClassDescriptor.cs 2006-06-09 22:59:49.000000000
> > +0200
> > +++ stetic.clean/libstetic/ClassDescriptor.cs   2006-06-24
> > 20:46:00.000000000 +0200
> > @@ -9,6 +9,20 @@
> >         
> >         public abstract class ClassDescriptor
> >         {
> > +               public struct ToolboxItemDefinitions
> > +               {
> > +                       public string Name;
> > +                       public string Category;
> > +                       public string Description;
> > +                       
> > +                       public ToolboxItemDefinitions (string aName,
> > string aCategory, string aDescription)
> > +                       {
> > +                               Name = aName;
> > +                               Category = aCategory;
> > +                               Description = aDescription;
> > +                       }
> > +               }
> 
> Why do you need this struct? class descriptors need a name, category and
> description, doesn't matter if they have been described using attributes
> or xml.
> 

I wouldn't need this struct if I would mix both approaches, true. But
since my approach is priority override, this struct provides:
- much cleaner way to clean up after moving to attributes (that's if
moving from objects.xml to attributes would occur) would be finished
- cleaner code (I could make three string properties, but that would be
much more confusing and complex to clean up after). It is the usual
priority override way.

> > (snip)
> 
> > diff -ruN stetic/libstetic/Registry.cs
> > stetic.clean/libstetic/Registry.cs
> > --- stetic/libstetic/Registry.cs        2006-03-15 16:40:40.000000000
> > +0100
> > +++ stetic.clean/libstetic/Registry.cs  2006-06-26 04:43:16.000000000
> > +0200
> > @@ -1,5 +1,6 @@
> >  using System;
> >  using System.Collections;
> > +using System.ComponentModel;
> >  using System.Reflection;
> >  using System.Xml;
> >  using System.Xml.Xsl;
> > @@ -7,15 +8,41 @@
> >  namespace Stetic {
> >         public static class Registry {
> >  
> > +               public delegate void PaletteGroupsChanged();
> > +
> > +               public class ControlGroup {
> > +                       public string Name;
> > +                       public string DisplayName;
> > +                       public int Ref;
> > +                       
> > +                       public ControlGroup (string name, string
> > displayname)
> > +                       {
> > +                               Name = name;
> > +                               DisplayName = displayname;
> > +                               Ref = 1;
> > +                       }
> > +               }
> 
> If the class is public, please use properties, not public fields.
> 

Fixed

> > +               
> > +               public static void DoRegisterWidgetLibrary
> > (WidgetLibrary library, bool register)
> > +               {
> > +                       if (library is AssemblyWidgetLibrary) {
> > +                               AssemblyWidgetLibrary asm =
> > (AssemblyWidgetLibrary) library;
> > +                               bool onlygroup, group = false;
> > +                               bool ctrl, cat, desc = false;
> > +                               string name, category, description =
> > "";
> > +                               foreach (System.Type type in
> > asm.Asm.GetExportedTypes()) {
> > +                                       Attribute[] attrs =
> > Attribute.GetCustomAttributes (type);
> > +                                       onlygroup = ((type.IsAbstract
> > == true) || (type.IsNotPublic == true));
> > +                                       
> > +                                       ctrl = false; cat = false;
> > desc = false; group = false;
> > +                                       name = ""; category = "";
> > description = "";
> > +                                       foreach (Attribute attr in
> > attrs) {
> > +                                               if (attr is
> > ToolboxItemAttribute) {
> > +                                                       name =
> > type.Name;
> > +                                                       if
> > (attr.Equals(ToolboxItemAttribute.Default) == true) {
> > +                                                               ctrl =
> > true;
> > +                                                               if
> > ((type.IsAbstract == true) || (type.IsNotPublic == true))
> > +                                                                       continue;
> > +                                                       }
> > +                                                       else
> > +                                                               //
> > Group specifiying class has to be abstract
> > +                                                               if
> > ((attr.Equals(ToolboxItemAttribute.None) == true) && (type.IsAbstract
> > == true)) {
> > +                                                                       group = true;
> > +                                                                       ctrl = true;
> > +                                                               }
> > +                                                       if ((ctrl ==
> > false) && (group == false))
> > +                                                               continue;
> > +                                               }
> > +                                               if (attr is
> > CategoryAttribute) {
> > +                                                       cat = true;
> > +                                                       category =
> > (attr as CategoryAttribute).Category;
> > +                                               }
> > +                                               if (attr is
> > DescriptionAttribute) {
> > +                                                       desc = true;
> > +                                                       description =
> > (attr as DescriptionAttribute).Description;
> > +                                               }
> > +                                       }
> > +
> > +                                       if (ctrl == true)
> > +                                               if (group == true) {
> > +                                                       if
> > (description == "")
> > +                                                               description = category;
> > +                                                       if (register
> > == true)
> > +                                                               AddControlGroup (category, description);
> > +                                                       else
> > +                                                               RemoveControlGroup (category);
> > +                                               }
> > +                                               else
> > +                                                       if
> > (ControlGroupExists(category) == false) {
> > +                                                               category = "unsorted";
> > +                                                               if
> > (ControlGroupExists("unsorted") == false)
> > +                                                                       AddControlGroup ("unsorted", "[Unsorted]");
> > +                                                       }
> > +                               }
> > +                       }
> > +               }
> 
> All the previous method is specific to AssemblyWidgetLibrary, so it
> should be moved to that class.
> 

Was deleted at the same time I took M.J.'s suggested approach.

> And please, instead of "if (group==true)..." or "if (group==false)..."
> use either "if (group)..." or "if (!group)...".
> 

Fixed.

> Lluis.
> 

m.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: corrected_complete.patch
Type: text/x-patch
Size: 12956 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/monodevelop-list/attachments/20060701/b28b963e/attachment-0001.bin 


More information about the Monodevelop-list mailing list