Re: strange code in ediff-hook.el

[Michael Kifer]

> address@hidden (Michael Kifer) writes:
> 
>   > > address@hidden (Michael Kifer) writes:
>   > > 
>   > >   > > Eli Zaretskii <address@hidden> writes:
>   > >   > > 
>   > >   > >   > > From: Dan Nicolaescu <address@hidden>
>   > >   > >   > > Date: Sat, 08 Mar 2008 12:11:15 -0800
>   > >   > >   > > 
>   > >   > >   > > 
>   > >   > >   > > Is this code in ediff-hook.el useful:
>   > >   > >   > > 
>   > >   > >   > > (if purify-flag
>   > >   > >   > >     () ; if dumping, autoloads are set up in loaddefs.el
>   > >   > >   > >   ;; if the user decides to load this file, set up autoloads
>   > >   > >   > >   ;; compare files and buffers
>   > >   > >   > >   (autoload 'ediff "ediff" "Compare two files." t)
>   > >   > >   > >  [....]
>   > >   > >   > > 
>   > >   > >   > > It's hard to see why... What am I missing? 
>   > >   > 
>   > >   > This is from the times when ediff was not part of emacs. It is also 
> useful
>   > >   > for debugging (if the menus will ever be changed, which last time 
> happened
>   > >   > years ago).
>   > > 
>   > > So you'd not object if those autoloads are removed? 
>   > 
>   > Why remove? I said it is useful for debugging if the menues will need to 
> be
>   > changed.
> 
> When adding a function to the global menu, one has to make sure that
> either the function is defined by default or it is autoloaded.  Why
> keep 100+ lines of redundant code for something that is well known and
> pretty easy to check (just check if the new menu actually works). 

Sorry, the purify-flag code was not for the menus but for XEmacs.
I need to find out how things are done in XEmacs these days and see if this
code is still needed.


>   > >   > >   > > Incidentally all ediff*.el files have some strange 
> require/featurep/load
>   > >   > >   > > code.  Is that stuff really needed?
>   > >   > 
>   > >   > Are you talking about the compiler pacifiers? This reduces the 
> number of
>   > >   > compile-time warnings. At least used to.
>   > > 
>   > > There are other ways of avoiding warnings (nowadays we zero byte-compile
>   > > warnings when bootstrapping emacs).  I'll get rid of that code.
>   > 
>   > What does it have to do with bootstrapping? 
> 
> Bootstrapping tends to show more warnings that when compiling interactively.
> 
>   > I am talking about debugging.  When there are many warnings due to
>   > some files not being loaded, it becomes hard to spot real problems.
> 
> Please explain why won't just using `require' work? (or (eval-when-compile 
> (require )))

Is "require" being used during the compilation?
When this code was written, require was definitely not used at compile time.

> BTW, when trying to removed that type of code, it showed that 
> the defsubst ediff-get-session-number-at-pos is used before being
> defined.  So all that loading can hides other bugs...

This technique has drawbacks, but it proved to be very useful.
The above is not a bug. This function will simply not be inlined.
In Ediff, inlining was useful 12 years ago, but it is not worth
the trouble any more. So I'll replace it with a defun.

>   > > Also is this autoload still needed? 
>   > > 
>   > > ;; This autoload is useless in Emacs because ediff-hook.el is dumped 
> with
>   > > ;; emacs, but it is needed in XEmacs
>   > > ;;;###autoload
>   > > (if (featurep 'xemacs)
>   > >     (progn
>   > >       (defvar ediff-menu
>   > >       '("Compare"
>   > >         ["Two Files..."  ediff-files t]
>   > >           ["Two Buffers..." ediff-buffers t]
>   > >     [snip...]
>   > 
>   > 
>   > That seems to be not necessary any more.
> 
> Then, please remove it.  There are 2 instances of the same comment.


I take it back. This was needed for XEmacs when this code was written.
I will find out if this is not necessary any more or if there is a better
way.