gEDA-dev: PCB <-> HID interface stuff

[der Mouse <mouse@xxxxxxxxxxxxxxxxxxxxxx>]

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	       mouse@xxxxxxxxxxxxxxxxxxxxxx
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


_______________________________________________
geda-dev mailing list
geda-dev@xxxxxxxxxxxxxx
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev