Working on PCB, I've found I have some questions, issues, whatever you want to call them, about the interface between PCB proper and the HIDs.
I've been noting things with XXX comments in my code as I find them. This is based on those comments. I'd value any thoughts any of you have about any of these. - Why is HID_Action.name not marked const? (With all the consting issues, I'm really talking about the pointer target type.) This makes it impossible to initialize it with a string constant while still doing proper const poisoning. - It would be useful to have a call PCB promises to make to the HID after argument parsing and before any "operational" calls. As it is, I have to just look at the order I'm getting the calls in and assume the end of parse_arguments is the right place for last-chance setup. (I have some setup to do after commandline parsing and before things go operational; do_expert is much too late.) - set_color is documented as taking certain special names, like "drill" and "erase". But I find no doc on what others, if any, there are, nor on what the semantics of even those two are. - draw_arc takes _two_ radius values. Is it expected to draw ellipses, or what? If ellipses, are the angles given in the squashed coordinate system of the ellipse, or unsquashed? And, what units are the angles in? - add_timer's interface design looks good, but there's a nasty gotcha lurking; the auto-save code assumes the .ptr member of a timer's returned hidval is not a nil pointer. (Why design a nice general interface and then violate it like that? Odd. Two different people, perhaps?) - watch_file takes a "condition" argument whose possible values are not documented. - Terminator issues aside, confirm_dialog has problems - it's documented as a "generic yes/no dialog", but at least one call to it passes three button label strings and expects to get 0, 1, or 2 back. The doc also doesn't give any indication what order the button label strings should be in. - Why isn't confirm_dialog()'s first argument consted? - Why aren't title and msg, to report_dialog, consted? - prompt_for returns a char *, holding a string. The doc gives no indication who is responsible for the memory making up the string after the return. The existing HIDs vary; some return a static buffer, others malloc space - but the callers free the result; I can only assume that nobody's tested certain HIDs with amloc()s that don't like freeing static buffers. - prompt_for's callers are prepared for the returned pointer to be nil, but the doc doesn't say what the semantics of a nil return are. - Why aren't prompt and defstr to prompt_for consted? - The comment on HID_FILESELECT_IS_TEMPLATE says the call should return a "file template", but gives no idea what such a thing is. (Fortunately for me, HID_FILESELECT_READ is the only one of the three bits that's actually used.) - fileselect() takes a history tag pointer. It's not clear whether history should be associated with the pointer or what it points to; based on the examples, I would assume fileselect() is expected to treat it as a NUL-terminated string and use the string's text as the tag, but I'd prefer to see that documented. - fileselect()'s returned string's allocation semantics are undocumented, as for prompt_for above. - Nil returns from fileselect() are handled by callers, but the semantics of a nil return are undocumented. - Why isn't flags, to fileselect, unsigned? Am I the only person who prefers to use unsigned types for bitmasks? - Why does attribtue_dialog exist? As far as I can tell it's never referred to anywhere; did I miss something? - hid.h says PCB expect the GUI to provide six actions: PCBChanged, RouteStylesChanged, NetlistChanged, LayersChanged, LibraryChanged, and Busy. However, it does not describe the semantics expected of them; also, the list is incomplete - it is missing PointCursor. - It's not clear whom the comments in hid.h are addressed to. When they say to do something, it's not always clear whether it's something PCB must do or something the HID must do. I'm not providing patches, because most of these are things I don't understand. Once I have them figured out (whether with or without help), I'll be able to provide updated comments for hid.h. /~\ The ASCII der Mouse \ / Ribbon Campaign X Against HTML [EMAIL PROTECTED] / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B _______________________________________________ geda-dev mailing list [email protected] http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev
