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

[Stefan Monnier]

>> Why not just use admin/FOR-RELEASE? It's easier to look in just one place.
> This is up to the maintainers.  I thought while the port is settling down
> and there are a lot of issues, it might be good to keep them from
> cluttering up the admin file.

Actually, I think it can be placed into admin/FOR-RELEASE.  Thanks to
the outline mode, anybody bothered by this Emacs.app section can fold it
away from view ;-)

> Actually, the Carbon and Windows ports put these defs in source code,
> macfns.c and w32fns.c respectively.  Keeping them in a separate data  file
> seems a little cleaner, and also fits well with the way color  lists are
> handled in NeXTstep.  But the defs can be moved into source  code if that
> is preferred.

I have no preference, except that merging them into a single file sounds
like a good idea (to the extent possible, obviously: I haven't looked
at it at all).

> This is so users can enter colors in numeric format, such as ARGBD0FFFFFF.
> The NS port interprets such formats to allow alpha  specification.

Can't similar "uncompletable colors" be specified in X11 (with format
"#RRGGBB" or somesuch)?  Maybe your change should be applied to
more platforms?

In any case, we need a comment in that code to explain why
`require-match' is set or not, and when.

> This is not done by default, only when ns-extended-platform-support is
> turned on.  And it makes only very minor modifications, for purpose of
> enhancing platform consistency.  Anyway, if this is retained, one option
> would be to move it out into a separately-loaded file (not included in
> dumped emacs).  Another would be to manually do what easymenu does (but
> this would be ugly).

Maybe a separate file would be a good choice.  Better yet: make it work
(as much as possible) for non-NS platforms as well, in case someone used
to those modifications wants to use them under w32, X11, ...

> Here I aped what the Carbon port does just above these lines.  If it's
> wrong, which example should I follow?

The dependencies seem harmless for other platforms, so you can just
remove the #ifdef.  Any removal of CPP tricks in src/Makefile.in is
good, since it gets us one step closer to the point where we can get rid
of the cpp processing step.

>> Index: src/frame.c
>> ...
>> Fselect_window (XFRAME (frame)->selected_window, Qnil);
>> 
>> +#ifdef NS_IMPL_COCOA
>> +  /* term gets no other notification of this */
>> +  if (for_deletion)
>> +    Fraise_frame(Qnil);
>> +#endif
>> 
>> Why isn't his needed for other platforms too?

> I don't know.  I would be happy to get rid of it if I knew.

Please explain as precisely as you can what is the problem you're trying
to solve with the above code.

> Are there other options besides FIXME?  I use PENDING to flag something that
> is not necessarily a bug or even needing change, but that needs to be
> considered.  What about TODO?

I use FIXME for those as well.

> Yes, I was working to make this only happen when ns-extended-platform-
> support-mode is on, but it's a bit tricky since this only gets called once.

With the new where-is-preferred-modifier, this should now be easier.

> Yes, hash_put, hash_remove, etc. are all used in those libs.
> Only hash_remove is declared so publicly within emacs though.

Actually, I see hash_remove in lisp.h, but it seems not to be used
outside of fns.c so it should probably be made static.


        Stefan