Re: StdProp overhaul: broken plug-ins

[Cyrille Chepelov <chepelov calixo net>]

Le jeu, aoû 16, 2001, à 12:00:47 +0200, Hans Breuer a écrit:

[advertisement for GObject]

I'm a bit wary of GObject.... Given the effort which will be required to
convert to GObject anyway, shouldn't we just convert (rewrite) in C++ ? Or
maybe in Python.

Oh well. As I've already said, I'm not developing for an unstable platform
(no, I mean GTK2 <grin/>). So I'll look at GObject, but when it's about to
be mainstream, not sooner  (but this "let's make Pythonic objects in C"
thing sure looks strange, and perhaps a bit awkward !)

> Ok you're right with the switch statement, I'll probably need to 
> understand Quarks some time ...

I understand them as a trivial somehow guaranteed unique hash of a string.
You build them with g_quark_from_string() if the string has to be copied, or
g_quark_from_static_string() if you're sure the string won't be
de-allocated. Then, you can safely compare the quarks, since a GQuark is an
int.

> > > Property     *object_find_prop (Object *obj, const char* name);
> >
> > Problem with the prototype you've proposed is that this returns a single
> > property. And objects expect lists (GPtrArray) of properties....
> But can an object really have two indepent properties which share
> the same name ??

You could build a bogus property descriptor list with twice the same
property name, but Realistically, No. But obj->ops->[sg]et_props() 
expect a GPtrArray of (Property *). In fact, with the (to be renamed, see
below) prop_list_singleton() routine, this is not an issue.

> You don't mean the singleton from 'Design Patterns', do you?

Whoops, of course not. I mean, list of a single element (I think I used a
lowercase "s" letter, though). This routine will have to be renamed, I
don't want to cause any confusion on this particular topic. Mmmm... Is
prop_list_of_single(prop) clear enough ?

> > (yes, this is almost your proposal with a few additions). The code above
> > becomes :
> >    Property *p;
> >    char* name = PyString_AsString(key);
> >    p = object_find_prop (self->object, name,0);  
>
> what flags do you have in mind ? Setting a patameter for future
> unknown additions appears to be rather odd.

No. It's not for future unknown additions, it's because the name (type) and
flags parameters are used by make_new_property(), which will synthetise a
property descriptor. Some plug-ins will want to make synthetic visible
properties, some won't. Hmmm. In this function, we can get rid of it, and
instanciate the property through its pdesc->ops->new_prop(pdesc->ops,pdtpp_true)
class method rather than rebuild a dynamic propdesc.

If you're working on this before I remove the flags parameter, please do,
just pass a 0 as the last argument of make_new_property() in
object_prop_from_name_type().

> > You'll have noted that object_prop_by_name is almost object_find_prop(). If 
> > it can't find a matching property name in the object's description, it'll 
> > return NULL. If it can, it will (as you assumed) ask the object to fill the 
> > property. 
> Why mot simply return the (ref counted) pointer of the object. 

What object ? If it's the first argument of object_prop_by_name(), I don't
see the point....

> Ok there is currently no ref counting, but the Python plug-in
> will crash on deleting whole Dia objects under it's feet
> anyway. But the object reference is needed.

You already have it, since you pass it to object_prop_by_name().

> > In this case, it's almost trivial. Simply define a set routines matching
> >   PYTHONTYPE (*PythonFromPropFunc) (const Property *prop);
> >
> > (actually, something like:
> > static PYTHONTYPE pyint_from_charprop(const CharProperty *prop);
> > static PYTHONTYPE pyint_from_intprop(const IntProperty *prop); 
> >   etc...)
> Can't see how this would help clarifing the code. In the whole
> Python plug-in there is exactly one place where the de-marshalling
> is needed ...

Simple. The demarshaling code itself (the contents of the switch) is
garbage. And, having a switch requires you an integral type. Solution is to
hide the garbage an replace the non-integral type by a pointer to a function
(which is integral). Oh well, you saw my point, anyway. And there's always
the solution of a (string)PropertyType to
(integral)PyDiaPropTypeSwitchCookie mapping through a GHash. Probably less
trouble, indeed.

> > lots of prototypes and braces, but less casts... And this trivial garbage
> > can easily be hidden in a separate compilation unit (that's the strategy of
> > all these lib/prop_*.c) 
> ... maybe it should provide the type conversion functions than, too ?

Hmm. I'm not sure what the "it" in your last sentence refers to.

<assuming referred="lib/prop_*.c">
It does, in the form of the GetFromOffsetsFunc. In the case of PyDia,
setting up a PropOffset structure and calling GFO on it (for each different
type) is probably much more hassle than peeking into a semi-public
structure.
</assuming>

> > > +#ifdef THE_PROP_TYPE_ID_IS_INTEGRAL
> > >  #define CASE_STR(s) case PROP_TYPE_##s : tname = #s; break;
> >
> > yeeech !
> I always try to balance between design theory and pragmatism :-)

hehe...

        -- Cyrille

-- 
Grumpf.