gEDA-dev: Broken slotting

[Peter Clifton]

On Tue, 2007-09-11 at 06:40 -0700, Steve Meier wrote:
> If anyone cares, I would like to propose an API for attributes.

Good to see your thoughts, I'd not got as far as sorting out the
necessary functions yet.

I do have a patch which culls ATTRIB entirely... as by the time you
remove the linked list, and the copied_to stuff moves into the parent
OBJECT, all you are left is an OBJECT pointer. Keeps thing more
simple ;)

Of course, we still need an API to deal with attribute OBJECTS..
I've added some comments below:


> ATTRIB *s_attrib_new( );
No point having an empty attribute really - invoke with name and value.

> void s_attrib_init(ATTRIB *this_attrib, char *name, char *value, BOOL
> show_name, BOOL show_value,
>            POINT location, int rotation);
Presumably combined with the above to make an attribute constructor.

> void s_attrib_free(ATTRIB* this_attrib);
Currently freeing an "ATTRIB" just detaches the attribute object from
its parent, and removes the ATTRIB entry from the parent OBJECT's
attribs list. By the time you cull ATTRIB and link directly to child
attribute objects, this is not so much a "free" but "detach".

> //----------------------------------------------------------------
> // duplicate an attribute.
> // caller is responsible for destroying ruterned attribute
> ATTRIB *s_attrib_duplicate(ATTRIB *this_attrib);
Not necessary... the o_..._copy() API can take care of this when copying
the actual attribute TEXT object. Then we just need to attach to a
parent. I don't think this makes a lot of sense without knowing where
(if at all) we want to attach the copied attribute.

> //-------------------------------------------------------------------
> // write an attribute to the open file pointer
> 
> void s_attrib_write(ATTRIB *this_attrib, FILE *fp);
Fine.

> //----------------------------------------------------------------
> // print an attribute report.
> void s_attrib_print (ATTRIB *this_attrib);
Fine, although we might want a wrapper which prints details of an
attribute list. The current attribute print function reports for a while
component's worth of attributes.

> //----------------------------------------------------------------
> // compare two attribs to see if the have equivellent values
> BOOL s_attrib_compare(ATTRIB *first_attrib, ATTRIB *second_attrib);
Could do - where is it desired in the code though? I'm not saying it
won't be useful - just there is no point adding it unless multiple code
use-cases would benefit from it.

> //----------------------------------------------------------------
> // search the attributes of an object for one named by name
> // return the value of the attribute of the first one found
> // be sure caller free's return value
> char *s_attrib_search_name(GList *list, char *name, int counter);
What is the GList of? Attributes? Objects? How does counter work in
either of those cases? 'n'th object with an "x" attribute. Does this
work as intended if objects have multiple "x" attributes?

> //----------------------------------------------------------------
> // for a complex object, find the value of the slot attribute
> // be sure caller frees string
> char *s_attrib_search_slot(OBJECT *object);
No. Special case-tastic. If anything, we could make an {s|o}_slot.c and
define slotting API there. Slots _use_ attributes, but aren't necessary
in the attribute API its-self.

> //----------------------------------------------------------------
> // for a complex object, find the number of slots the object has
> int s_attrib_search_numslots(OBJECT *object);
Again, special case slot stuff.

> //----------------------------------------------------------------
> // for a complex object, find which slot is the default slot
> int s_attrib_search_default_slot(OBJECT *object);
Again, special case slot stuff.

> //----------------------------------------------------------------
> // return the pin type object that matches the sequence pin_number
> // the list is a list of pins
> // caller should not destroy the returned object
> OBJECT *s_attrib_search_pinseq(GList *list, int pin_number);
Again, special case slot stuff really.

> //----------------------------------------------------------------
> // from a list of attributes find the slot deffinition string
> // that coresponds to the slot number be sure caller frees string
> char *s_attrib_search_slotdef(GList *attrib_list, int slotnumber);
Again, special case slot stuff.

> //-----------------------------------------------------------------
> // for an attribute GString determine its name and value
> // if the name and value are non-null strings
> // return TRUE else return false
> // set the returned name and value function call perameters as well
> BOOL s_attrib_get_name_value(GString *string, char **name, char **value);
I'd stick with char* string personally.

> //----------------------------------------------------------------
> // search the attributes of an object for oned named by name
> // return the value of the attribute of the first one found
> // be sure caller free's return value
> char *s_attrib_search_name_single(OBJECT *object, char *name);
How is this different to s_attrib_search_name(..) above? I guess its
possible one will be used to implement the other.

> //----------------------------------------------------------------
> // find the text object that matches the string
> // the caller should NOT destroy the returned OBJECT
> OBJECT *s_attrib_search_string_list(GList *list, char *string);
I don't like the name - implies we're searching for one of a list of
strings. I'm not 100% sure where this API would be used.

> //----------------------------------------------------------------
> // find the pin number for a pin from a complex object with respect to a
> slot number
> // be sure caller free's return value
> char *s_attrib_pin_sequence_slot_pinnumber(OBJECT *object, OBJECT
> *pin_object, int slot);
Special case slot stuff.

> //-----------------------------------------------------------------------------------------
> // find the nth occurence starting from zero of the search_for string in
> the list of attribs
> // it is the responsibility of the caller to free the returned string
> // returned string is the found value
> 
> gchar *s_attrib_search_string_partial(GList *attribs, gchar *name, gchar
> *partial, int nth);
The name isn't clear, nor is the functionality. How is this different to
s_attrib_search_string_list(...)? Could we combine their functions?

> //-------------------------------------------------------------------
> // Search for the nth occurance of the attribute, starts from ZERO!
> // zero being the first occurance
> // caller is responsible for freeing returned string
> 
> gchar * s_attrib_search_outside(GList *attribs, gchar *name, int
> nth_occurance_value);
Isn't clear what this function does, or why the other API can't do this.

> //-------------------------------------------------------------------
> // test to see if a specific pin exists in a net attrib
> 
> BOOL s_attrib_net_attrib_contains_pin(ATTRIB *net_attrib, gchar *pinnumber);
I'm not sure what code would use this, anyhow - it seems like a special
case for some function other than {s|o}_attrib.c

> //-------------------------------------------------------------------
> // use the viewer application graphic routines to draw the attribute in
> the desired graphics context
> 
> void s_attrib_draw(ATTRIB *this_attrib, BOOL color_mode, hidGC gc);
Not in libgeda, although we do want some generic mechanism for notifying
the client (gschem, gattrib, other..) of updates. This isn't attribute
specific though.

> //-------------------------------------------------------------------
> // Create a box that contains the complex object
> 
> BOX *s_attrib_get_bounds(ATTRIB *this_attrib);
> 
> //-------------------------------------------------------------------
> // move an attrib in world coordinants
For now, attributes are just TEXT objects, and I wasn't proposing to
change that right now.

> void s_attrib_translate(ATTRIB *this_attrib, POINT vector);
As above.

I have considered the possibility of attaching attributes right to the
circuit entities in a non-graphical sense. ("name"="value") tagged
against a hierarhical sub-circuit, net etc..

This would allow attributes to exist without being hidden text on the
schematic page (which might be handy for over-riding them in a
hierarchy, or storing other meta-data). The graphical text on the
schematic would then just be a pointer (or path) to the attribute it
displays, along with flags to determine whether it formats the name,
value or both. Hidden would still be possible of course.

Anyway - I wasn't proposing to change anything in this regard, just to
clean up the existing API so when you read the function name - you know
what it is doing - and that there aren't another 5 functions sitting
like cyber-squatters with nearly identical sounding names.

Regards,

-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)