mh-e/mh-acros.el advices `require' incorrectly

mh-e/mh-acros.el advices `require' incorrectly

[Kenichi Handa]

This is in the latest CVS.

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.

Ex:
(require 'mh-e)
(require 'enriched) => enriched
(require 'enriched) => nil

How about this change?

*** mh-acros.el	13 Jan 2006 10:27:43 +0900	1.13
--- mh-acros.el	13 Jan 2006 16:45:04 +0900	
***************
*** 152,161 ****
  
  (defadvice require (around mh-prefer-el activate)
    "Modify `require' to load uncompiled MH-E files."
!   (or (featurep (ad-get-arg 0))
!       (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
!            (load (format "%s.el" (ad-get-arg 0)) t t))
!       ad-do-it))
  
  (defmacro mh-assoc-ignore-case (key alist)
    "Check if KEY is present in ALIST while ignoring case to do the comparison.
--- 152,162 ----
  
  (defadvice require (around mh-prefer-el activate)
    "Modify `require' to load uncompiled MH-E files."
!   (if (or (featurep (ad-get-arg 0))
! 	  (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! 	       (load (format "%s.el" (ad-get-arg 0)) t t))
! 	  ad-do-it)
!       (setq ad-return-value (ad-get-arg 0))))
  
  (defmacro mh-assoc-ignore-case (key alist)
    "Check if KEY is present in ALIST while ignoring case to do the comparison.

---
Kenichi Handa
handa@m17n.org

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

Kenichi Handa <handa@m17n.org> writes:

> This is in the latest CVS.
>
> 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.
>
> Ex:
> (require 'mh-e)
> (require 'enriched) => enriched
> (require 'enriched) => nil
>
> How about this change?
>
> *** mh-acros.el	13 Jan 2006 10:27:43 +0900	1.13
> --- mh-acros.el	13 Jan 2006 16:45:04 +0900	
> ***************
> *** 152,161 ****
>   
>   (defadvice require (around mh-prefer-el activate)
>     "Modify `require' to load uncompiled MH-E files."
> !   (or (featurep (ad-get-arg 0))
> !       (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
> !            (load (format "%s.el" (ad-get-arg 0)) t t))
> !       ad-do-it))
>   
>   (defmacro mh-assoc-ignore-case (key alist)
>     "Check if KEY is present in ALIST while ignoring case to do the comparison.
> --- 152,162 ----
>   
>   (defadvice require (around mh-prefer-el activate)
>     "Modify `require' to load uncompiled MH-E files."
> !   (if (or (featurep (ad-get-arg 0))
> ! 	  (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
> ! 	       (load (format "%s.el" (ad-get-arg 0)) t t))
> ! 	  ad-do-it)
> !       (setq ad-return-value (ad-get-arg 0))))
>   
>   (defmacro mh-assoc-ignore-case (key alist)
>     "Check if KEY is present in ALIST while ignoring case to do the comparison.

Satyaki,

What do you (and others) think about this?

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Stefan Monnier]

> *** mh-acros.el	13 Jan 2006 10:27:43 +0900	1.13
> --- mh-acros.el	13 Jan 2006 16:45:04 +0900	
> ***************
> *** 152,161 ****
  
>   (defadvice require (around mh-prefer-el activate)
>     "Modify `require' to load uncompiled MH-E files."
> !   (or (featurep (ad-get-arg 0))
> !       (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
> !            (load (format "%s.el" (ad-get-arg 0)) t t))
> !       ad-do-it))
  
For what it's worth:
We usually try to avoid defadvice within Emacs and I think for
good reasons.  Now, some elisp packages do you use defadvice, but they only
only do it when it's absolutely necessary.  The above advice seem to be just
a minor convenience hack to making recompiling occasionally easier.

I'd rather try and come up with a generic fix for the problem.
I personally use the patch below for now.


        Stefan


--- orig/lisp/emacs-lisp/bytecomp.el
+++ mod/lisp/emacs-lisp/bytecomp.el
@@ -1640,6 +1640,12 @@
   ;; Force logging of the file name for each file compiled.
   (setq byte-compile-last-logged-file nil)
   (let ((byte-compile-current-file filename)
+        ;; Prefer source files over compiled files.  This is so that when
+        ;; several files are changed and recompiled, each new file is
+        ;; properly recompiled with the new macros in the other new files.
+        (load-suffixes (sort (copy-sequence load-suffixes)
+                             (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
+                                             (string-match "\\.el\\b" s1)))))
 	(set-auto-coding-for-load t)
 	target-file input-buffer output-buffer
 	byte-compile-dest-file)

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> *** mh-acros.el	13 Jan 2006 10:27:43 +0900	1.13
>> --- mh-acros.el	13 Jan 2006 16:45:04 +0900	
>> ***************
>> *** 152,161 ****
>   
>>   (defadvice require (around mh-prefer-el activate)
>>     "Modify `require' to load uncompiled MH-E files."
>> !   (or (featurep (ad-get-arg 0))
>> !       (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
>> !            (load (format "%s.el" (ad-get-arg 0)) t t))
>> !       ad-do-it))
>   
> For what it's worth:
> We usually try to avoid defadvice within Emacs and I think for
> good reasons.  Now, some elisp packages do you use defadvice, but they only
> only do it when it's absolutely necessary.  The above advice seem to be just
> a minor convenience hack to making recompiling occasionally easier.
>
> I'd rather try and come up with a generic fix for the problem.
> I personally use the patch below for now.
>
>
>         Stefan
>
>
> --- orig/lisp/emacs-lisp/bytecomp.el
> +++ mod/lisp/emacs-lisp/bytecomp.el
> @@ -1640,6 +1640,12 @@
>    ;; Force logging of the file name for each file compiled.
>    (setq byte-compile-last-logged-file nil)
>    (let ((byte-compile-current-file filename)
> +        ;; Prefer source files over compiled files.  This is so that when
> +        ;; several files are changed and recompiled, each new file is
> +        ;; properly recompiled with the new macros in the other new files.
> +        (load-suffixes (sort (copy-sequence load-suffixes)
> +                             (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
> +                                             (string-match "\\.el\\b" s1)))))
>  	(set-auto-coding-for-load t)
>  	target-file input-buffer output-buffer
>  	byte-compile-dest-file)

Is there a problem with your committing this patch? Then we can get
rid of the defadvice and make everyone happy ;-).

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Richard M. Stallman]

    --- orig/lisp/emacs-lisp/bytecomp.el
    +++ mod/lisp/emacs-lisp/bytecomp.el
    @@ -1640,6 +1640,12 @@
       ;; Force logging of the file name for each file compiled.
       (setq byte-compile-last-logged-file nil)
       (let ((byte-compile-current-file filename)
    +        ;; Prefer source files over compiled files.  This is so that when
    +        ;; several files are changed and recompiled, each new file is
    +        ;; properly recompiled with the new macros in the other new files.
    +        (load-suffixes (sort (copy-sequence load-suffixes)
    +                             (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
    +                                             (string-match "\\.el\\b" s1)))))
	    (set-auto-coding-for-load t)
	    target-file input-buffer output-buffer
	    byte-compile-dest-file)

That creates an inconsistency.  If it were a reliable solution to the
problem, that could justify the inconsistency.  But it isn't reliable,
because if the file require'd was already loaded in bytecode outside
the compiler, this won't reload it.

Thus, if we want a reliable solution, it has to be something else.

For the same reason, the advice in mh-e.el is not a good solution
(even disregarding that it is a bad thing for parts of Emacs to
advise other parts).

Re: mh-e/mh-acros.el advices `require' incorrectly

[Stefan Monnier]

>     --- orig/lisp/emacs-lisp/bytecomp.el
>     +++ mod/lisp/emacs-lisp/bytecomp.el
>     @@ -1640,6 +1640,12 @@
>        ;; Force logging of the file name for each file compiled.
>        (setq byte-compile-last-logged-file nil)
>        (let ((byte-compile-current-file filename)
>     +        ;; Prefer source files over compiled files.  This is so that when
>     +        ;; several files are changed and recompiled, each new file is
>     +        ;; properly recompiled with the new macros in the other new files.
>     +        (load-suffixes (sort (copy-sequence load-suffixes)
>     +                             (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
>     +                                             (string-match "\\.el\\b" s1)))))
> 	    (set-auto-coding-for-load t)
> 	    target-file input-buffer output-buffer
> 	    byte-compile-dest-file)

> That creates an inconsistency.  If it were a reliable solution to the
> problem, that could justify the inconsistency.  But it isn't reliable,
> because if the file require'd was already loaded in bytecode outside
> the compiler, this won't reload it.

Indeed, it's just a minor convenience hack.

> For the same reason, the advice in mh-e.el is not a good solution
> (even disregarding that it is a bad thing for parts of Emacs to
> advise other parts).

100% agreement.


        Stefan

Re: mh-e/mh-acros.el advices `require' incorrectly

[Richard M. Stallman]

    The origianl `require' returns FEATURE even if it is already
    provided, but the adviced `require' returns nil in such a
    case.

Packages in Emacs are not supposed to advise other parts of Emacs.
So really the right thing to do is get rid of this advising.
It is not clean for `require' to work differently in a certain
part of Emacs.

Bill, what was the reason for this?

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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

>     The origianl `require' returns FEATURE even if it is already
>     provided, but the adviced `require' returns nil in such a
>     case.
> 
> Packages in Emacs are not supposed to advise other parts of Emacs.
> So really the right thing to do is get rid of this advising.
> It is not clean for `require' to work differently in a certain
> part of Emacs.
> 
> Bill, what was the reason for this?

I trust you've seen Satyaki's and Stefan's messages by now. I'm happy to
remove the defadvice as soon as the problem it works around has been
solved. Perhaps we can install Stefan's patch or one like it.

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

RE: mh-e/mh-acros.el advices `require' incorrectly

[Drew Adams]

    > 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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Satyaki Das]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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.

Re: mh-e/mh-acros.el advices `require' incorrectly

[Kenichi Handa]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Richard M. Stallman]

    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?

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Andreas Schwab]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Stefan Monnier]

> 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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Richard M. Stallman]

    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.

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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.

Re: mh-e/mh-acros.el advices `require' incorrectly

[Richard M. Stallman]

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.

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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.

Re: mh-e/mh-acros.el advices `require' incorrectly

[David Kastrup]

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

Re: mh-e/mh-acros.el advices `require' incorrectly

[Bill Wohler]

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