Re: [anjuta-devel] New project interface and introspection

[Abderrahim Kitouni <a kitouni gmail com>]

Hello,
              في ن، 06-09-2010 عند 23:11 +0200 ، كتب Sébastien Granjoux:
> Hi,
>
> Here is a first proposal:
>
> /**
>   * SECTION:ianjuta-project-backend
>   * @title: IAnjutaProjectBackend
>   * @short_description: Interface for creating new project
>   * @see_also:
>   * @stability: Unstable
>   * @include: libanjuta/interfaces/ianjuta-project-backend.h
>   *
>   */
> interface IAnjutaProjectBackend
> {
>       #include "libanjuta/anjuta-project.h"
>       
>       /**
>        * ianjuta_project_backend_new_project:
>        * @obj: Self
>        * @file: Project file
>        * @err: Error propagation and reporting
>        *
>        * Get a new project root node. The file object can be a file or
>        * or a directory.
>        *
>        * Return value: An object derived from GbfProject
from AnjutaProjectNode :-) and you may need to add (transfer full): I'm
not sure what's the default. (btw, I hope you don't mind my comments
about the comments)
>        */
>       AnjutaProjectNode* new_project (GFile *file);
>
>       
>       /**
>        * ianjuta_project_backend_probe:
>        * @obj: Self
>        * @file: Project file
>        * @err: Error propagation and reporting
>        *
>        * Check if the directory contains a project supported by this
>        * backend. The file object can be a directory or a file.
>        *
>        * Return value: 0 if the project is invalid and > 0 if the
>        * project is valid. Bigger number means more specific
>        * project. By example, a backend considering all files as sources
>        * and directory as group should return a low number as it will
>        * match any directory. A backend checking if the directory contains
>        * a Makefile should return higher number.
>        */
>       gint probe (GFile *directory);
> }
>
> /**
>   * SECTION:anjuta-project
>   * @title: AnjutaProjectNode
>   * @short_description: The base object for Anjuta project nodes.
>   * @see_also:
>   * @stability: Unstable
>   * @include: libanjuta/anjuta-project.h
>   *
>   */
>
>       /**
>        * AnjutaProjectNode::changed:
>        * @obj: Self
>        * @node: Updated node.
>        *
>        * This signal is emitted when the node is changed.
>        */
>       void ::changed (AnjutaProjectNode *node);
>
>
>       /* Here is a list of virtual methods needed */
>
>       /**
>        * anjuta_project_node_load:
>        * @obj: Self
>        * @err: Error propagation and reporting
>        *
>        * Load or reload a project node. This function is used to load the
>        * complete project by calling it on the root node created by
>        * ianjuta_project_backend_new_project.
>        *
>        * Return value: The loaded node, NULL if error
>        */
>       AnjutaProjectNode *anjuta_project_node_load (AnjutaProjectNode *obj, 
> GError *err);
Is the (re)load really supposed to create a new node object? (and if so,
you probably want to add a (transfer full) annotation. Or is just that
reloading a node could reload a parent node instead?

>       /**
>        * anjuta_project_node_save:
>        * @obj: Self
>        * @err: Error propagation and reporting
>        *
>        * Save the project node and all its children.
>        *
>        * Return value: The saved node, NULL if error
>        */
>       AnjutaProjectNode *anjuta_project_node_save (AnjutaProjectNode *obj, 
> GError *err);
This returns the node that actually got saved, right? how about
transferring ownership? I guess it shouldn't.

>       /**
>        * anjuta_project_node_new:
>        * @parent: Self
>        * @type: Node type
>        * @file: Optional file object for the node
>        * @name: Optional name for the node
>        * @err: Error propagation and reporting
>        *
>        * Create a new child node
>        *
>        * Return value: The new node, NULL if error
>        */
>       AnjutaProjectNode *anjuta_project_node_new (AnjutaProjectNode *obj, 
obj is the parent, right? in this case how references are supposed to
work? (which object should own the reference to the other)

> AnjutaProjectNodeType type, GFile *file, const gchar *name, GError *err);
Don't forget to register these as (GObject) properties.

>       /**
>        * anjuta_project_node_set_property:
>        * @obj: Self
>        * @property: Property
>        * @value: Value
>        * @err: Error propagation and reporting
>        *
>        * Change a properties on node.
>        *
>        * Return value: FALSE if some properties cannot be set.
there is only one property AFAICS
>        */
>       gboolean anjuta_project_node_set_property (AnjutaProjectNode *obj, 
> AnjutaProjectProperty* property, const gchar *value, GError *err);


> It looks simpler than the current interface, but there still several issues:
Yes, /me likes simpler :-)
> 1. For ianjuta_project_backend_new_project, I think a name argument is 
> not needed. I suppose a project will always correspond to a file or a 
> directory else we can add a name argument or use the GFile to contain 
> this name as it doesn't need to be a valid file.
I think this is fine. I can't think of a case where the GFile won't be a
directory (or cannot somehow get a directory from it).


> 2. For the changed signal, it's probably needed to emit this on the 
> changed node and all its parents. So an user can connect only on the 
> root node to get all changes on the project. But in this case, we need 
> to get a node argument telling which is node that trigger the change. Or 
> is there another way to get this ?
I don't see any other way.

> 3. The backends are expected to implement an object derived from 
> AnjutaProjectNode. I hope it's supported by every bindings, I think only 
> interfaces are not always supported. I prefer to use a base classe 
> instead of an interface, because I can implement in the base 
> AnjutaProjectNode class, all GNode functions (adding children, listing 
> them...).
1. this should definitely be a class (probably abstract), since it will
have some common data (corresponding gfile, links to other nodes, etc.)
2. We need not to worry about bindings IMHO : if it works with
PyGObject, it will eventually work with others.


> 4. Not all node support the load and save function. In this case we can 
> return the node (one parent) which has really been saved or just an 
> error. It is the case of target in autotools backend.
That's fine for me. Maybe just add a default implementation that does that.

> 5. The name argument of the anjuta_project_node_new is useful when the 
> node doesn't correspond to a file, like target in autotool project. I 
> could use only a GFile argument and set it with the corresponding name 
> as a GFile doesn't always correspond to a valid file. What do you think ?
I feel this is abusing the GFile, so I prefer a separate name. And since
it's going to be subclassed, I think even the new function is 

> 6. I haven't decided if anjuta_project_node_new, insert the node as a 
> child of its parent. It seems more obvious but I'm afraid it could cause 
> trouble with thread, I have to check.
I think it can also cause trouble with reference counts. And since it's
not going to be much used (g_object_{get,set}_data will only be used in
simple cases, in all others a subclass is going to be written anyway),
so I think there should be a separate add_child with (transfer full) for
C convenience.

> 7. I need to change the way properties are handled. Probably I can have 
> two lists, one for default properties and one for custom properties. The 
> properties object will correspond to the data. Like node, it could need 
> some specific backend information, so I could make it a GObject too but 
> there is probably more properties. I will think about this later.
I think it's not necessary if we are going to "remove"
AnjutaProjectProperty and use AnjutaProjectPropertyInfo everywhere

> 8. The free function is gone but it is replaced by g_object_unref. I'm 
> not convince it's a big improvement. Moreover I'm afraid in Python, 
> there is no guarantee that it is called. It doesn't matter if you use it 
> to free memory, but it's a problem if you want to update a file in it.
No, it should be fine. It's a bit complicated when it's actually called
in python (because python uses both ref counting and a GC), but it
eventually gets called.

Anyway, I like this new interface, we must take care of reference
handling and registering properties and we should be fine.

Regards,
Abderrahim