sp_status_display()/sp_status_clear()
I notice that sp_status_display() was introduced a couple days ago, and that it is used like this:
static SPDrawAnchor * sp_draw_anchor_new (SPDrawContext *dc, SPCurve *curve, gboolean start, gdouble dx, gdouble dy) { SPDrawAnchor *a;
sp_status_display(g_strdup_printf ("Creating anchor at %g %g", dx, dy)); ... return a; }
Admittedly, the "original" would have been overly complex. The thing is, I think we have to be careful not to simplify the wrong things.
For example, why did we need the last parameter to sp_view_set_status() anyway? It's never actually used. It seems a good candidate to remove.
Also, shifting half the burden of memory management off is not a bad thing, but why not get both ends? Otherwise to people other than authors of sp_status_display(), it looks like a memory leak.
[ rule of thumb: the module that allocates memory should almost always be the one to free it ]
Why not change sp_view_set_status() or add an sp_view_set_statusf() so that it allocated and formatted the string internally?
sp_view_set_statusf(SP_VIEW(SP_EVENT_CONTEXT_DESKTOP (dc)), "Creating anchor at %g %g", dx, dy);
I realize SP_VIEW() and SP_EVENT_CONTEXT_DESKTOP() are awkward, but they are a cost of doing business in C.
When we move to C++, we can start doing away with lengthy accessor names and we can remove the need to explicitly cast to superclasses:
dc->get_view()->set_statusf("Creating anchor at %g %g", dx, dy);
I have introduced an sp_view_set_statusf() and a corresponding sp_view_clear_status() to CVS HEAD. I would appreciate it if you would use them in place of sp_status_display() and sp_status_clear().
sp_view_set_statusf_va() can also be used to implement other *_set_statusf() methods in other places (see sp_view_set_statusf()'s implementation for an example).
-mental
participants (1)
-
MenTaLguY