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

[Adrian Robert]

On Jul 28, 2008, at 3:00 PM, Dan Nicolaescu wrote:

> 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.

Hi Jason,

Do you have any objection to moving/renaming w32-load-color-file to  
xfaces.c so the NS port can use it?  Also, does it need to be exposed  
to lisp (syms_of...)?  It's not called from anywhere in the emacs tree  
lisp anyway.

Finally, how necessary is the hard-coded set of xcolors definitions in  
w32fns.c?  It's not OK just to abort if this file is not found in data- 
directory?