unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* RE: mh-e/mh-acros.el advices `require' incorrectly
       [not found] <87oe2gez38.fsf@olgas.newt.com>
@ 2006-01-13 21:04 ` Drew Adams
  2006-01-13 22:11   ` Bill Wohler
  0 siblings, 1 reply; 20+ messages in thread
From: Drew Adams @ 2006-01-13 21:04 UTC (permalink / raw)
  Cc: mh-e-devel

    > The origianl `require' returns FEATURE even if it is already
    > provided, but the adviced `require' returns nil in such a
    > case.  So, once mh-acros.el is loaded, a code depending on
    > the return value of `require' stops working.
    >
    > How about this change?

    What do you (and others) think about this?

Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
don't think we should change the return value of `require' at all. It has
acted this way for a very long time, and there is undoubtedly code that
depends on this behavior (at least some external libraries). Besides, we
certainly should not make such a change just before a release.



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-13 21:04 ` mh-e/mh-acros.el advices `require' incorrectly Drew Adams
@ 2006-01-13 22:11   ` Bill Wohler
  2006-01-13 23:36     ` Satyaki Das
                       ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-13 22:11 UTC (permalink / raw)
  Cc: emacs-devel, mh-e-devel

Drew Adams <drew.adams@oracle.com> wrote:

>     > The origianl `require' returns FEATURE even if it is already
>     > provided, but the adviced `require' returns nil in such a
>     > case.  So, once mh-acros.el is loaded, a code depending on
>     > the return value of `require' stops working.
>     >
>     > How about this change?
> 
>     What do you (and others) think about this?
> 
> Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
> don't think we should change the return value of `require' at all. It has
> acted this way for a very long time, and there is undoubtedly code that
> depends on this behavior (at least some external libraries). Besides, we
> certainly should not make such a change just before a release.

Unless you load MH-E, require does not change. If I understand
correctly, Kanichi's patch addresses your concern (about the return
value of require changing). Here's the ChangeLog for that change:

    2004-08-23  Satyaki Das  <satyaki@theforce.stanford.edu>

	* mh-acros.el (mh-require-cl): Remove unneeded autoloads.
	(require): Add an advice to the function so that at compile time
	the uncompiled file is loaded. This avoids compilation problems
	when built in the Emacs tree.

Satyaki, just out of curiosity, do you remember what the problems were?
Would we be able to remove the require defadvice if we removed the
circular dependencies?

-- 
Bill Wohler <wohler@newt.com>  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.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-13 22:11   ` Bill Wohler
@ 2006-01-13 23:36     ` Satyaki Das
  2006-01-14  1:39       ` Bill Wohler
  2006-01-14  0:29     ` Kenichi Handa
  2006-01-14 16:14     ` Richard M. Stallman
  2 siblings, 1 reply; 20+ messages in thread
From: Satyaki Das @ 2006-01-13 23:36 UTC (permalink / raw)
  Cc: Drew Adams

On 1/13/06, Bill Wohler <wohler@newt.com> wrote:
> Drew Adams <drew.adams@oracle.com> wrote:
>
> >     > The origianl `require' returns FEATURE even if it is already
> >     > provided, but the adviced `require' returns nil in such a
> >     > case.  So, once mh-acros.el is loaded, a code depending on
> >     > the return value of `require' stops working.
> >     >
> >     > How about this change?
> >
> >     What do you (and others) think about this?
> >
> > Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
> > don't think we should change the return value of `require' at all. It has
> > acted this way for a very long time, and there is undoubtedly code that
> > depends on this behavior (at least some external libraries). Besides, we
> > certainly should not make such a change just before a release.
>
> Unless you load MH-E, require does not change.

Even if you load MH-E, require isn't adviced since mh-acros.el is
never loaded at run  time unless you load it yourself.

>                                                                         If I understand
> correctly, Kanichi's patch addresses your concern (about the return
> value of require changing). Here's the ChangeLog for that change:

Kenichi's patch won't cause any problems for MH-E.

>
>     2004-08-23  Satyaki Das  <satyaki@theforce.stanford.edu>
>
>         * mh-acros.el (mh-require-cl): Remove unneeded autoloads.
>         (require): Add an advice to the function so that at compile time
>         the uncompiled file is loaded. This avoids compilation problems
>         when built in the Emacs tree.
>
> Satyaki, just out of curiosity, do you remember what the problems were?
> Would we be able to remove the require defadvice if we removed the
> circular dependencies?

That advice forces the load of src .el file.  Suppose you have three
files a.el, b.el and c.el and the compiled files a.elc, b.elc and
c.elc.  Suppose that there is a macro foo in c.el that is used in
a.el.  Now let us make a change to macro foo.  Then if we recompile
all the files in the directory then they are compiled in the lexical
order.  So when a.el is getting compiled, require will load the c.elc
and get the old definition of the macro.  The advice above avoids
this.

A better solution would be to use Stefan's change in bytecomp.el.  If
it were checked in, we can drop the advice to require and it will make
things nicer elsewhere too.

Satyaki


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_idv37&alloc_id\x16865&op=click

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-13 22:11   ` Bill Wohler
  2006-01-13 23:36     ` Satyaki Das
@ 2006-01-14  0:29     ` Kenichi Handa
  2006-01-14 16:14     ` Richard M. Stallman
  2 siblings, 0 replies; 20+ messages in thread
From: Kenichi Handa @ 2006-01-14  0:29 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

In article <837.1137190309@olgas.newt.com>, Bill Wohler <wohler@newt.com> writes:

>>  Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
>>  don't think we should change the return value of `require' at all. It has
>>  acted this way for a very long time, and there is undoubtedly code that
>>  depends on this behavior (at least some external libraries). Besides, we
>>  certainly should not make such a change just before a release.

> Unless you load MH-E, require does not change. 

Unfortunately, even if we don't load MH-E explicitly, now,
just compiling some file in emacs/lisp/mh-e/*.el change the
behaviour of `require'.

> If I understand correctly, Kanichi's patch addresses your
> concern (about the return value of require changing).

Yes.

---
Kenichi Handa
handa@m17n.org

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-13 23:36     ` Satyaki Das
@ 2006-01-14  1:39       ` Bill Wohler
  0 siblings, 0 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-14  1:39 UTC (permalink / raw)


Satyaki Das <satyakid@gmail.com> wrote:

> On 1/13/06, Bill Wohler <wohler@newt.com> wrote:
> > Drew Adams <drew.adams@oracle.com> wrote:
> >
> > >     > The origianl `require' returns FEATURE even if it is already
> > >     > provided, but the adviced `require' returns nil in such a
> > >     > case.  So, once mh-acros.el is loaded, a code depending on
> > >     > the return value of `require' stops working.
> > >     >
> > >     > How about this change?
> > >
> > >     What do you (and others) think about this?
> > >
> > > Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
> > > don't think we should change the return value of `require' at all. It has
> > > acted this way for a very long time, and there is undoubtedly code that
> > > depends on this behavior (at least some external libraries). Besides, we
> > > certainly should not make such a change just before a release.
> >
> > Unless you load MH-E, require does not change.
> 
> Even if you load MH-E, require isn't adviced since mh-acros.el is
> never loaded at run  time unless you load it yourself.
> 
> >                                                                         If I understand
> > correctly, Kanichi's patch addresses your concern (about the return
> > value of require changing). Here's the ChangeLog for that change:
> 
> Kenichi's patch won't cause any problems for MH-E.

This means you can check in your patch, Kenichi ;-).

Thanks very much for the explanation, Satyaki.

-- 
Bill Wohler <wohler@newt.com>  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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-13 22:11   ` Bill Wohler
  2006-01-13 23:36     ` Satyaki Das
  2006-01-14  0:29     ` Kenichi Handa
@ 2006-01-14 16:14     ` Richard M. Stallman
  2006-01-14 23:00       ` Bill Wohler
  2 siblings, 1 reply; 20+ messages in thread
From: Richard M. Stallman @ 2006-01-14 16:14 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

    Unless you load MH-E, require does not change. If I understand
    correctly, Kanichi's patch addresses your concern (about the return
    value of require changing).

MH-E should not advise `require'; nothing should.
Another solution is necessary.

Which files are trying to force loading in source form?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-14 16:14     ` Richard M. Stallman
@ 2006-01-14 23:00       ` Bill Wohler
  2006-01-14 23:16         ` Andreas Schwab
  2006-01-15 23:07         ` Richard M. Stallman
  0 siblings, 2 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-14 23:00 UTC (permalink / raw)
  Cc: mh-e-devel

"Richard M. Stallman" <rms@gnu.org> writes:

> MH-E should not advise `require'; nothing should.
> Another solution is necessary.

We only had the problem our workaround addresses when compiling in the
Emacs tree. We do not have any problems when compiling in our own
environment because we remove all MH-E .elc files before compiling.

What do people think about my updating lisp/Makefile.in to remove
lisp/mh-e/*.elc if and only if lisp/mh-e/*.el changes?

-- 
Bill Wohler <wohler@newt.com>  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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-14 23:00       ` Bill Wohler
@ 2006-01-14 23:16         ` Andreas Schwab
  2006-01-15  0:26           ` Bill Wohler
  2006-01-15 23:07         ` Richard M. Stallman
  1 sibling, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2006-01-14 23:16 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

Bill Wohler <wohler@newt.com> writes:

> "Richard M. Stallman" <rms@gnu.org> writes:
>
>> MH-E should not advise `require'; nothing should.
>> Another solution is necessary.
>
> We only had the problem our workaround addresses when compiling in the
> Emacs tree. We do not have any problems when compiling in our own
> environment because we remove all MH-E .elc files before compiling.

That's what `make bootstrap' does.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-14 23:16         ` Andreas Schwab
@ 2006-01-15  0:26           ` Bill Wohler
  2006-01-15  2:22             ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Bill Wohler @ 2006-01-15  0:26 UTC (permalink / raw)
  Cc: emacs-devel, mh-e-devel

Andreas Schwab <schwab@suse.de> wrote:

> Bill Wohler <wohler@newt.com> writes:
> 
> > "Richard M. Stallman" <rms@gnu.org> writes:
> >
> >> MH-E should not advise `require'; nothing should.
> >> Another solution is necessary.
> >
> > We only had the problem our workaround addresses when compiling in the
> > Emacs tree. We do not have any problems when compiling in our own
> > environment because we remove all MH-E .elc files before compiling.
> 
> That's what `make bootstrap' does.

You can't expect everyone to run make bootstrap when they change one
lisp file.

-- 
Bill Wohler <wohler@newt.com>  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.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-15  0:26           ` Bill Wohler
@ 2006-01-15  2:22             ` Stefan Monnier
  0 siblings, 0 replies; 20+ messages in thread
From: Stefan Monnier @ 2006-01-15  2:22 UTC (permalink / raw)
  Cc: mh-e-devel

> You can't expect everyone to run make bootstrap when they change one
> lisp file.

Yet, that's how we've worked for many years now.
In practice the problems you describe are infrequent and managed by hand or
by `make bootstrap'.


        Stefan


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-14 23:00       ` Bill Wohler
  2006-01-14 23:16         ` Andreas Schwab
@ 2006-01-15 23:07         ` Richard M. Stallman
  2006-01-16  1:29           ` Bill Wohler
  1 sibling, 1 reply; 20+ messages in thread
From: Richard M. Stallman @ 2006-01-15 23:07 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

    What do people think about my updating lisp/Makefile.in to remove
    lisp/mh-e/*.elc if and only if lisp/mh-e/*.el changes?

That won't help with M-x byte-recompile.  The right solution is to
make the graph of requires acyclic, and put the macro file at the
start of the alphabet.  Given the way the software in fact works,
that should be totally reliable.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-15 23:07         ` Richard M. Stallman
@ 2006-01-16  1:29           ` Bill Wohler
  2006-01-17  3:24             ` Richard M. Stallman
  0 siblings, 1 reply; 20+ messages in thread
From: Bill Wohler @ 2006-01-16  1:29 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

Richard M. Stallman <rms@gnu.org> wrote:

>     What do people think about my updating lisp/Makefile.in to remove
>     lisp/mh-e/*.elc if and only if lisp/mh-e/*.el changes?
> 
> That won't help with M-x byte-recompile.  The right solution is to
> make the graph of requires acyclic, and put the macro file at the
> start of the alphabet.  Given the way the software in fact works,
> that should be totally reliable.

Thank you. I think I'm going to go ahead and bite the bullet and do just
that. I've appended a proposal I sent to the MH-E developers today.
While I don't expect you to understand MH-E, perhaps you can let me know
whether your gut says "good idea" or "bad idea" and why.

(mh-customize.el holds all of our defcustom and defface calls.
mh-loaddefs.el is the internal MH-E analog of loaddefs.el.)

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

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.

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.

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

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

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.

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

-- 
Bill Wohler <wohler@newt.com>  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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-16  1:29           ` Bill Wohler
@ 2006-01-17  3:24             ` Richard M. Stallman
  2006-01-17  4:20               ` Bill Wohler
  0 siblings, 1 reply; 20+ messages in thread
From: Richard M. Stallman @ 2006-01-17  3:24 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

I don't understand 100% of the plan, but it looks like a good plan
overall.

    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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-17  3:24             ` Richard M. Stallman
@ 2006-01-17  4:20               ` Bill Wohler
  2006-01-17  9:18                 ` David Kastrup
  2006-01-29 19:22                 ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Bill Wohler
  0 siblings, 2 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-17  4:20 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

Richard M. Stallman <rms@gnu.org> 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.

-- 
Bill Wohler <wohler@newt.com>  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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-17  4:20               ` Bill Wohler
@ 2006-01-17  9:18                 ` David Kastrup
  2006-01-17 17:19                   ` Bill Wohler
  2006-01-29 19:22                 ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Bill Wohler
  1 sibling, 1 reply; 20+ messages in thread
From: David Kastrup @ 2006-01-17  9:18 UTC (permalink / raw)
  Cc: mh-e-devel

Bill Wohler <wohler@newt.com> writes:

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

You are aware that this sounds like smoldering duct tape all over?
I am not at all acquainted with the involved code, but if you feel it
is best described with those words...

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: mh-e/mh-acros.el advices `require' incorrectly
  2006-01-17  9:18                 ` David Kastrup
@ 2006-01-17 17:19                   ` Bill Wohler
  0 siblings, 0 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-17 17:19 UTC (permalink / raw)
  Cc: emacs-devel, mh-e-devel

David Kastrup <dak@gnu.org> wrote:

> Bill Wohler <wohler@newt.com> writes:
> 
> > 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.
> 
> You are aware that this sounds like smoldering duct tape all over?
> I am not at all acquainted with the involved code, but if you feel it
> is best described with those words...

The code has roots from the early 80's so it's long past the smoldering
point ;-). But the bones are strong; it just needs a renovation ;-).

I usually describe it in these words: http://mh-e.sourceforge.net/

-- 
Bill Wohler <wohler@newt.com>  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.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

^ permalink raw reply	[flat|nested] 20+ messages in thread

* MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly)
  2006-01-17  4:20               ` Bill Wohler
  2006-01-17  9:18                 ` David Kastrup
@ 2006-01-29 19:22                 ` Bill Wohler
  2006-01-29 21:10                   ` MH-E reorg Bill Wohler
  2006-01-30 18:45                   ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Richard M. Stallman
  1 sibling, 2 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-29 19:22 UTC (permalink / raw)


Bill Wohler <wohler@olgas.newt.com> wrote:

> Richard M. Stallman <rms@gnu.org> 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 <wohler@newt.com>  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.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: MH-E reorg
  2006-01-29 19:22                 ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Bill Wohler
@ 2006-01-29 21:10                   ` Bill Wohler
  2006-01-30 18:45                   ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Richard M. Stallman
  1 sibling, 0 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-29 21:10 UTC (permalink / raw)


Bill Wohler <wohler@olgas.newt.com> wrote:

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

I forgot to point out that because lisp/Makefile.in changed, you first
have to run ./configure.

Apologies for the inconvenience.

-- 
Bill Wohler <wohler@newt.com>  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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly)
  2006-01-29 19:22                 ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Bill Wohler
  2006-01-29 21:10                   ` MH-E reorg Bill Wohler
@ 2006-01-30 18:45                   ` Richard M. Stallman
  2006-01-30 19:24                     ` Bill Wohler
  1 sibling, 1 reply; 20+ messages in thread
From: Richard M. Stallman @ 2006-01-30 18:45 UTC (permalink / raw)
  Cc: mh-e-devel, emacs-devel

Thanks for making this clean.  Certainly please install it.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly)
  2006-01-30 18:45                   ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Richard M. Stallman
@ 2006-01-30 19:24                     ` Bill Wohler
  0 siblings, 0 replies; 20+ messages in thread
From: Bill Wohler @ 2006-01-30 19:24 UTC (permalink / raw)
  Cc: emacs-devel, mh-e-devel

Richard M. Stallman <rms@gnu.org> wrote:

> Thanks for making this clean.  Certainly please install it.

Thank you. Done.

-- 
Bill Wohler <wohler@newt.com>  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.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2006-01-30 19:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <87oe2gez38.fsf@olgas.newt.com>
2006-01-13 21:04 ` mh-e/mh-acros.el advices `require' incorrectly Drew Adams
2006-01-13 22:11   ` Bill Wohler
2006-01-13 23:36     ` Satyaki Das
2006-01-14  1:39       ` Bill Wohler
2006-01-14  0:29     ` Kenichi Handa
2006-01-14 16:14     ` Richard M. Stallman
2006-01-14 23:00       ` Bill Wohler
2006-01-14 23:16         ` Andreas Schwab
2006-01-15  0:26           ` Bill Wohler
2006-01-15  2:22             ` Stefan Monnier
2006-01-15 23:07         ` Richard M. Stallman
2006-01-16  1:29           ` Bill Wohler
2006-01-17  3:24             ` Richard M. Stallman
2006-01-17  4:20               ` Bill Wohler
2006-01-17  9:18                 ` David Kastrup
2006-01-17 17:19                   ` Bill Wohler
2006-01-29 19:22                 ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Bill Wohler
2006-01-29 21:10                   ` MH-E reorg Bill Wohler
2006-01-30 18:45                   ` MH-E reorg (was: mh-e/mh-acros.el advices `require' incorrectly) Richard M. Stallman
2006-01-30 19:24                     ` Bill Wohler

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).