Anybody mind this being applied to the 21.5 branch? This moves read_bitmap_data_from_file into glyphs-shared.c. This definition is used even if Xmu is present on the system, so everybody reads .xbm files the same way. This code was duplicated in xmu.c if !HAVE_XMU, glyphs-gtk.c and glyphs-msw.c. I'm taking the hint ben. :)

-------------------------------------------------------------------------------- bitmap.diffName: bitmap.diff Type: Plain Text (text/plain) -------------------------------------------------------------------------------- -bp -- Ceterum censeo vi esse delendam

Ben Wing <ben(a)666.com&gt; writes: > "William M. Perry" wrote: > > > > Anybody mind this being applied to the 21.5 branch? This moves > > read_bitmap_data_from_file into glyphs-shared.c. This definition is used > > even if Xmu is present on the system, so everybody reads .xbm files the > > same way. > > > > This code was duplicated in xmu.c if !HAVE_XMU, glyphs-gtk.c and > > glyphs-msw.c. > > > > I'm taking the hint ben. :) > > thank you thank you thank you! I was looking at the redisplay and event code, and it is pretty close, but every single damn one of the function names is different, or has different argument orderings between X and GTK. Do you think it is worth having a gigantic set of macros in event-gtk.h and event-Xt.h and one common event-xish.c? I'm not sure if it would make things better or not. -bp -- Ceterum censeo vi esse delendam

I would really hate to make redisplay-xish.c look like:

I suppose that doing the ifdefs first would make it glaringly obvious which things should be turned into device methods.

Ben Wing <ben(a)666.com&gt; 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