MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly)

[Bill Wohler]

Bill Wohler <address@hidden> wrote:

> Richard M. Stallman <address@hidden> wrote:
> 
> > I don't understand 100% of the plan, but it looks like a good plan
> > overall.
> 
> Thanks for the feedback.
> 
> >     2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
> >        go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
> >        We would then create files for each of the modes. We now already have
> >        mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
> >        renaming mh-comp.el to mh-letter.el for consistency. Move
> >        mh-modify to mh-funcs.el where it belongs.
> > 
> >     6. Limit scope of variables to just the file in which it is defined.
> >        If necessary, provide access to variables via functions that are
> >        found in mh-loaddefs.el.
> > 
> > I think these two are be necessary to solve the present problem, and
> > could substantially increase the amount of change.  It might be better
> > not to do these changes now.
> 
> Maybe. I'm investigating it right now since it seems that every day we
> uncover a new problem associated with the current mess. It's extremely
> brittle. The changes will either work or they won't. If they do, I think
> it would actually add to the stability of the release to check in the
> changes. I'll let you know how it goes.

Hi Richard,

I finished the reorganization of MH-E over a week ago, and I'm extremely
pleased with the results. The developers and a few users have been smoke
testing a tarball of my work directory and there are no known issues
(the only issue revealed by the smoke test was a small Emacs 21 issue
which was quickly fixed).

I'm sorry this didn't come up sooner in the release cycle, but I'll go
ahead and check in the changes since I'm definitely going to use them
for the MH-E 8.0 release, the MH-E package isn't a critical part of
Emacs, and I think the Emacs 22 release will be more stable with few, if
any, risks.

Highlights:

1. Circular dependencies are gone.

2. defadvice of require is gone; shared macros have been moved to
   mh-acros.el. This means that you'll never get stale macro definitions
   making the defadvice unnecessary.

3. I was able to delete this code *within*
   mh-gnus-article-highlight-citation:

  ;; Requiring gnus-cite should have been sufficient. However for Emacs21.1,
  ;; recursive-load-depth-limit is only 10, so an error occurs. Also it may be
  ;; better to have an autoload at top-level (though that won't work because
  ;; of recursive-load-depth-limit). That gets rid of a compiler warning as
  ;; well.
  (unless mh-xemacs-flag
    (require 'gnus-art)
    (require 'gnus-cite))

4. I was able to delete this code in mh-utils.el:

 (eval-and-compile
   (defvar recursive-load-depth-limit)
   (if (and (boundp 'recursive-load-depth-limit)
            (integerp recursive-load-depth-limit)
            (< recursive-load-depth-limit 50))
       (setq recursive-load-depth-limit 50)))

5. I was able to delete eval-and-compile tricks as in the following
   around mh-show-font-lock-keywords:

    (eval-and-compile
      ;; Otherwise byte-compilation fails on
      ;; `mh-show-font-lock-keywords-with-cite'

6. XEmacs now compiles with just three warnings (from XEmacs macros
   which I can't do anything about). It used to compile with *hundreds*
   of warnings and a handful of errors too. (Note that MH-E has always
   compiled clean in GNU Emacs--it is a developer requirement.)

While the diffs will be large, they mostly represent functions moving
from one file to another. The functional groupings are now much more
cohesive and consistent and will be easier to maintain.

Developers should be aware that three files will be deleted so it might
be a good idea to remove lisp/mh-e/*.elc. The entry points are in
different files too so users of CVS MH-E will have to run "cd lisp; make
autoloads". You can accomplish both, of course, with "make bootstrap".
Nine new files were added.


If you're curious, here is the original plan with commentary:

> 1. Remove all (require 'mh-*) lines, move the provides in
>    mh-customize.el and mh-e.el to the end and start from scratch ;-).

Done.

> 2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
>    go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
>    We would then create files for each of the modes. We now already have
>    mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
>    renaming mh-comp.el to mh-letter.el for consistency. Move
>    mh-modify to mh-funcs.el where it belongs.

Done. While I moved mh-letter-mode into mh-letter.el, It still made
sense to keep mh-comp.el. I also pulled the limiting code out of
mh-seq.el into mh-limit.el and the threading code out of mh-seq.el into
mh-thread.el. In addition, I created mh-compat.el (for compatibility
defsubsts) and mh-tool-bar.el for the tool bar creation code.

> 3. Make mh-e.el a lean, mean entry point. Make it require mh-acros,
>    mh-customize, *and nothing else* within MH-E. Have it define pretty
>    much only mh-rmail, mh-smail, and mh-version (functions with the
>    ;;;###autoload cookie) as well as a few other generic globals such as
>    mh-xemacs-flag. The frequently-used commands would go into
>    mh-folder.el and the rest would go into mh-funcs.el.

I made mh-e.el lean and mean, but not exactly as I described. I moved
nearly all of the code into other more logical files, pulled in the
defcustoms from mh-customize.el, and then just enough code to get it to
compile, including mh-init.el and mh-exec.el (removing those three files
as a result). The only entry point left in mh-e.el is mh-version. I
moved the other entry points into their corresponding files (for
example, mh-smail into mh-comp.el and mh-rmail into mh-folder.el). The
result eliminates dependencies in mh-e.el, and reduces the amount of
code that needs to be loaded when running, say, just mh-smail.

> 4. Make mh-customize require mh-loaddefs to get the function definitions
>    it needs *and nothing else* within MH-E.

The content of mh-customize.el was incorporated into mh-e.el and
the file was removed.

> 5. Everything else then requires mh-e.el (implicitly getting
>    mh-customize and mh-loaddefs) *and nothing else* within MH-E (except
>    for perhaps variable-only files like mh-buffers.el and mh-scan.el).

Done.

> 6. Limit scope of variables to just the file in which it is defined.
>    If necessary, provide access to variables via functions that are
>    found in mh-loaddefs.el.

I did this in a couple of places where it was very clearly the right
thing to do, but left the vast majority of globals there were previously
spread across mh-e.el and mh-utils.el in mh-e.el.

> 7. Move all macros into mh-acros.el so that we can remove the defadvice
>    of require in mh-acros.el.

Done, except that I didn't move all macros, just those that were used
in more than one file.

-- 
Bill Wohler <address@hidden>  http://www.newt.com/wohler/  GnuPG ID:610BD9AD
Maintainer of comp.mail.mh FAQ and MH-E. Vote Libertarian!
If you're passed on the right, you're in the wrong lane.