emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Emacs.app (Cocoa/GNUstep port) release and feature list


From: Dan Nicolaescu
Subject: Re: Emacs.app (Cocoa/GNUstep port) release and feature list
Date: Sat, 24 Nov 2007 08:33:52 -0800

"Adrian Robert" <address@hidden> writes:

  > On 11/24/07, Dan Nicolaescu <address@hidden> wrote:
  > 
  > >     (progn
  > >       (load "emacs-lisp/easymenu")
  > >       (load "emacs-lisp/easy-mmode")
  > >       (load "view")
  > >       (load "help-mode")
  > >       (load "help-fns")
  > >       (load "emacs-lisp/advice")
  > >       (load "ns-mark-nav")
  > >       (load "paren")
  > >
  > > Are these acceptable to be preloaded in platform specific code? IMHO no.
  > 
  > Could you elaborate on this?  These are loaded because setup in ns-win
  > needs them.    Is there some detrimental effect in loading them now
  > that ns-win must be loaded pre-dump?

We are trying to minimize the number of files preloaded, if you look in
loadup.el no other platform loads these files, you'd need to justify the
need to load them.

emacs-lisp/advice will 100% be rejected, if you need different
functionality, just change the function, not advice it.

  > >  lisp/ns-grabenv.el                 |only
  > >
  > > Doesn't the Carbon port have the same problem that this file tries to
  > > solve? Does GNUstep have this problem? Is the ns-* name appropriate
  > > then?
  > 
  > You are right this solves a problem mainly encountered on the Mac
  > platform for the Cocoa port.  

Are you saying that the Carbon port does not have to deal with this issue.

  > However it currently names all its functions with the "ns-" prefix,
  > so it seemed inappropriate to use "mac-".  It could also have some
  > use on GNUstep or other platforms if the user edits his/her .cshrc
  > after starting emacs.  

That is a generic problem then, just post it here separately and see if
it will get included.

  > >  lisp/ns-mark-nav.el                |only
  > >
  > > This does not seem to be specific to the port, why not submit is 
separately?
  > 
  > It is generic functionality, it could be submitted separately if it is
  > of interest.

Please do that then.

  > >  lisp/term/ns-win.el
  > 
  > >    Are the deffaces really needed in this file?
  > 
  > ns-working-text-face is needed.  paren-face-match-light will be
  > removed along with mic-paren when/if a merge becomes imminent.

That's not very productive for the people reviewing this code: if
something won't be included, then drop it and keep it as local
customization, it won't force unnecessary extra work on people trying to
help with the merge.

BTW, I just noticed this: 
(makunbound 'yank-menu-length)
(defcustom yank-menu-length 40

Use customize-set-variable instead. But even that would need
justification for changing a default in a platform specific file.


  > > I think you posted the changes to lisp/progmodes/cc-*.el to this
  > > list. What keeps them from being checked in CVS? They are not related to
  > > this port, just unnecessary overhead for you...
  > 
  > They are still not accepted by the maintainer yet.  If they get
  > accepted I can take them out.  Otherwise I feel they should at least
  > be bundled as part of the port, because Objective-C is the primary
  > development language for both Mac and GNUstep.

Were the changes not reviewed or not accepted at all? If the former, I'd
advice you repost them here, CCing RMS. In general he keeps track of
submitted patches.

  > > The new #defines: HAVE_NS GNUSTEP COCOA COCOA_EXPERIMENTAL_CTRL_G
  > >
  > > Why not HAVE_GNUSTEP and HAVE_COCOA too?
  > 
  > HAVE_XX seems to be standard for the overall windowing system / port
  > being used: HAVE_XWINDOWS, HAVE_NTGUI.  GNUSTEP and COCOA are platform
  > indicators, like DARWIN, MAC_OS8, GNU_LINUX, or VMS.  

IMHO it is much cleaner to use the HAVE_ prefix for new code. Besides
COCOA is not such a widely known term for the people that work on
emacs...

  > I would use "MAC_OSX" instead of COCOA, but that is already used by
  > the Carbon port to mean essentially HAVE_CARBONGUI.

That sounds very confusing!

  > > Also it seems that !GNUSTEP is the same as COCOA. Why not just use one
  > > of them?
  > 
  > I'm fine to make this change if people think it's easier to read.

Yeah, the least amount of identifiers that one has to learn the meaning
for, the better.

  > > Don't do any #ifdef MULTI_KBOARD, just assume it is always defined.
  > 
  > I don't understand this.  There is lots of #ifdef MULTI_KEYBOARD in
  > the emacs code.

For historic reasons, and it is only needed in multi-platform code. Only
the DOS port still needs !MULTI_KBOARD.

The Cocoa port defines MULTI_KBOARD unconditionally, so there's no need
to deal with it in platform specific code, it's just noise.

  > > Can the icons be shared with the Carbon port?
  > 
  > Emacs.app does not include any icons.  Unless you're talking about the
  > main app icon.  For now I think it's best to use a different one, so
  > users know which version they are running.

On X11, we don't have different icons for running with different
toolkits...

  > > Is the nextstep/compile script really needed? Wouldn't
  > > ./configure && make
  > > work on this platform?
  > 
  > With a little work it would not be needed.  

It would be great to not have to have different build instructions for
different platforms. But that might be done after the merge too. You
might want to consult RMS on this.

  > > You probably know RMS won't allow this code to be checked in without
  > > proper ChangeLogs...
  > 
  > There is nextstep/ChangeLog reflecting everything during my
  > maintainership period.  I don't think anything from the years
  > previous to that will be available.

Please follow the example in other branches: place the ChangeLog files
in the appropriate directories and call them ChangeLog.cocoa. 

For new files you just need a single entry: New file. Every single
change to generic emacs code needs to have a ChangeLog entry describing
it, even if it was done before you maintained the code.  If you'd look
on the mailing list for the experience with merging other code, you'd
see that RMS won't allow merges to proceed until the ChangeLogs are
acceptable.


        --dan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]