Re: a review of the merge (Re: Emacs.app merged)

[Dan Nicolaescu]

Adrian Robert <address@hidden> writes:

  > On Jul 20, 2008, at 7:56 AM, Dan Nicolaescu wrote:
  > 
  > > Adrian Robert <address@hidden> writes:
  > >
  > >> On Jul 16, 2008, at 5:23 PM, Dan Nicolaescu wrote:
  > >>
  > >>> Stefan Monnier <address@hidden> writes:
  > >>>
  > >>> We probably need some common file for these functions (and the
  > >>> humongous
  > >>> x-colors list) to avoid all the duplication that is happening now.
  > >>
  > >> If the x-colors list were put in a common file, with RGB specs, then
  > >> each non-X GUI could share it at the cost of a few lines to iterate
  > >> through the list --e.g.:
  > >>
  > >> lisp var has a list of char *name, unsigned char r,g,b
  > >>
  > >> macfns.c:
  > >>  colormap_t *mac_color_map = malloc(length-of-list);
  > >>  foreach-list-element mac_color_map[i] = { RGB_TO_ULONG(r,g,b), name
  > >> };
  > >>
  > >> w32fns.c:
  > >> colormap_t *w32_color_map = malloc(length-of-list);
  > >> foreach-list-element w32_color_map[i] = { name, PALETTERGB(r,g,b) };
  > >>
  > >> nsterm.m:
  > >> NSColorList *cl = [[NSColorList alloc] init];
  > >> foreach-list-element [cl setColor:
  > >>        [NSColor colorWithCalibratedRed:r green: g blue:b alpha: 1.0]
  > >>    forKey: [NSString stringWithUTF8String: name]];
  > >
  > > Let's go one step at a time: please make the nsterm.m code use
  > > something
  > > like this.  After having some working code it would be easy to move
  > > the
  > > big color array definition into some sort of a common file.
  > 
  > OK.  Here is a patch that moves w32_load_color_file from w32fns.c to
  > x_load_color_file in xfaces.c, and uses it in nsterm.m.  xfaces made
  > the most sense because there are already cross-platform color handling
  > functions in there.

The X11 code does not need this, so it should at least be properly
protected by #ifdefs, and maybe have a name.  w32-load-color-file does
not seem to be used outside w32fns.c, so maybe it does not even need to
get exported.  All these should probably be discussed with the w32
maintainers.