Ben Wing <ben(a)666.com> writes:
the basic idea is to avoid large amounts of duplicated code.
Always a good policy.
it sounds like you have a lot of functions where nearly all the code
is
the same, except for a few function calls. I'd suggest judicious uses of
ifdefs. you might even want to combine redisplay-x.c and redisplay-gtk.c
into one file, with ifdefs. obviously the line between when you use
ifdefs and separate out into two entirely separate functions is a
judgment call; but i strongly believe that when you have any significant
shared logic, it should appear only once, even if writing it is trickier
due to the ifdefs. whenever you have the same code duplicated, it *will*
start rotting, and very quickly.
I would really hate to make redisplay-xish.c look like:
static int
common_text_width_single_run(struct face_cachel *cachel, struct textual_run *run)
{
Lisp_Object font_inst = FACE_CACHEL_FONT (cachel, run->charset);
struct Lisp_Font_Instance *fi = XFONT_INSTANCE (font_inst);
if (!fi->proportional_p)
{
return fi->width * run->len;
}
else
{
if (run->dimension == 2)
{
#if defined(HAVE_GTK)
return gdk_text_width_wc (FONT_INSTANCE_GTK_FONT (fi),
(GdkWChar *) run->ptr, run->len);
#elif defined(HAVE_X_WINDOWS)
return XTextWidth16 (FONT_INSTANCE_X_FONT (fi),
(XChar2b *) run->ptr, run->len);
#elif defined(HAVE_MS_WINDOWS)
abort ();
#endif
}
else
{
#if defined(HAVE_GTK)
return gdk_text_width (FONT_INSTANCE_GTK_FONT (fi),
(char *) run->ptr, run->len);
#elif defined(HAVE_X_WINDOWS)
return
#elif defined(HAVE_MS_WINDOWS)
SIZE size;
mswindows_set_dc_font (hdc, font_inst,
cachel->underline, cachel->strikethru);
GetTextExtentPoint32 (hdc, (char *) run->ptr, run->len, &size);
return (size.cx);
#endif
}
}
}
To me that is incredibly unreadable. Those just scream for device
methods. But is that something that is appropriate for 21.5?
you may well discover [and i believe this to be true] that a lot of
system-dependent stuff in redisplay-*.c should actually be abstracted
out, and you might consider doing that too -- but the ifdef approach
should be the first thing you do.
I suppose that doing the ifdefs first would make it glaringly obvious which
things should be turned into device methods.
I'm trying to concentrate on Emacs/W3 for a little while, but I'll see what
I can do.
-bp
--
Ceterum censeo vi esse delendam