problem with mh-e and 2002-08-20 change to font-core.el

problem with mh-e and 2002-08-20 change to font-core.el

[y]

It seems that this change:

2002-08-20  Richard M. Stallman  <rms@gnu.org>

	* font-core.el (font-lock-change-mode): New function.
	(font-lock-mode): Use font-lock-change-mode instead of
	font-lock-unfontify-buffer.
	(font-lock-maximum-size, font-lock-verbose): Add defvars.

Causes a problem with mh-e (if the mh-show buffer enables font-lock,
such as with global-font-lock-mode).  Specifically, this change causes
any visited message buffer to be marked as modified when mh-e attempts
to display the second (and subsequent) message in a folder, and so
triggers mh-e's ``Message 920 modified; flush changes? (yes or no) ''
behavior.  I've temporarily backed out this change in my local emacs to
verify that the problem goes away.

It seems that this change (removing the properties) is a good change,
and I recall it being discussed on emacs-devel, but I only lightly
skimmed the discussion, and I don't have any strong ideas about how to
resolve this conflict.

chad

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Luc Teirlinck]

Chad wrote:

   It seems that this change:

   2002-08-20  Richard M. Stallman  <rms@gnu.org>

	   * font-core.el (font-lock-change-mode): New function.
	   (font-lock-mode): Use font-lock-change-mode instead of
	   font-lock-unfontify-buffer.
	   (font-lock-maximum-size, font-lock-verbose): Add defvars.

   Causes a problem with mh-e (if the mh-show buffer enables font-lock,
   such as with global-font-lock-mode).  Specifically, this change causes
   any visited message buffer to be marked as modified when mh-e attempts
   to display the second (and subsequent) message in a folder, and so
   triggers mh-e's ``Message 920 modified; flush changes? (yes or no) ''
   behavior.  I've temporarily backed out this change in my local emacs to
   verify that the problem goes away.

   It seems that this change (removing the properties) is a good change,
   and I recall it being discussed on emacs-devel, but I only lightly
   skimmed the discussion, and I don't have any strong ideas about how to
   resolve this conflict.

   chad

I believe that the fundamental problem underlying this is that
changing text properties marks the buffer as being changed.  From a
"philosophical" perspective, this actually makes sense since text
properties are considered part of the text.  From a practical
perspective (which, in my opinion, is the only really relevant
perspective) however, it leads to very strange behavior.  This was
already true before Richard's change.  All Richard's change did was
provide some additional instances of such strange behavior.

Do:

emacs-21.3.50 -q --eval '(blink-cursor-mode 0)'

(The last command line argument has nothing to do with the problem,
but it prevents serious damage to the nervous system.)

Visit some file in fundamental-mode.  (The last specification is
probably irrelevant.)

Do:

M-: (put-text-property 1 10 'face '((:background "orange")))

The buffer is now modified.

First inconsistency: Do:

M-x revert-buffer

The buffer is now marked unmodified, but the orange background is
still there.

Redo:

M-: (put-text-property 1 10 'face '((:background "orange")))

This is not redundant (although I personally believe it should be),
the buffer is once again considered modified.

C-x C-c.

We get amply warned about the changed buffer.

Ignore all warnings and exit, without "saving" the buffer.

Start Emacs again and visit the file again.  There reportedly are
saved changes which we should consider recovering. There is indeed an
auto-save file, but diff shows it is identical to the actual file.
Moreover, there is no trace of the orange background.

All of this behavior actually would make sense if text properties were
saved to file.  For instance, in that case revert-buffer would erase
the orange background.

However, write-region-annotate-functions is still nil and
after-insert-file-functions only contains
after-insert-file-set-buffer-file-coding-system, which, in as far has
I can tell, has nothing to do with saving and restoring text
properties.  If text properties would be saved to file, then maybe
the behavior reported by Chad would no longer be a bug either.  (I can
not tell, since I do not use mh-e.)

Is somebody actually working on saving text properties to file?
Would such a feature be optional or non-optional?

Sincerely,

Luc.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Luc Teirlinck]

In my previous message on this subject, I may have overlooked the fact
that changes in text properties should mark the buffer modified and
should make backup files if write-region-annotate-functions is
non-nil.  As Stefan pointed out, it is probably easy to take care of
Chad's original problem by using higher level functions.  As I already
pointed out in my previous message, if a non-nil
write-region-annotate-functions is pretty soon scheduled to be the
default, then the behavior I described actually makes perfect sense.

Sincerely,

Luc.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Luc Teirlinck]

From my previous message:

   In my previous message on this subject, I may have overlooked the fact
   that changes in text properties should mark the buffer modified and
   should make backup files if write-region-annotate-functions is
   non-nil.

I meant auto-save files (instead of backup files).

Sincerely,

Luc.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

Text properties are part of the buffer contents, so it is proper that
changing them normally marks the buffer as changed.  Perhaps in this
particular case we should mark the buffer as unmodified again.

But the real issue is, why does that command cause a change in major
modes, and should it run this hook at all?

Re: problem with mh-e and 2002-08-20 change to font-core.el

[chad]

The major mode is changed in this case by an artifact of mh-e.  The
problem, as identified by another mh-e developer is that the buffer is
set unmodified a tad too early, and we never noticed before.

Thanks, everyone, for investigating this, and to Satyaki Das for finding
the problem.

chad

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

    The major mode is changed in this case by an artifact of mh-e.  The
    problem, as identified by another mh-e developer is that the buffer is
    set unmodified a tad too early, and we never noticed before.

Are you saying that a change will be installed in mh-e.el to fix this
problem?  I think so, but I cannot tell for certain.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Bill Wohler]

Richard Stallman <rms@gnu.org> writes:
>     The major mode is changed in this case by an artifact of mh-e.  The
>     problem, as identified by another mh-e developer is that the buffer is
>     set unmodified a tad too early, and we never noticed before.
> 
> Are you saying that a change will be installed in mh-e.el to fix this
> problem?  I think so, but I cannot tell for certain.

  [Sorry for the delay, I've been out of town.]

  I saw that this had been fixed in the current CVS source for mh-e.

  See my next message regarding mh-e releases and Emacs...

--
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: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

    Causes a problem with mh-e (if the mh-show buffer enables font-lock,
    such as with global-font-lock-mode).  Specifically, this change causes
    any visited message buffer to be marked as modified when mh-e attempts
    to display the second (and subsequent) message in a folder, 

Why does that action cause a change in major mode?

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Stefan Monnier]

>     Causes a problem with mh-e (if the mh-show buffer enables font-lock,
>     such as with global-font-lock-mode).  Specifically, this change causes
>     any visited message buffer to be marked as modified when mh-e attempts
>     to display the second (and subsequent) message in a folder, 

I think that if we used (font-lock-mode -1) or some such higher-level
command the problem would disappear since they are careful to save/restore
the modification state of the buffer.


	Stefan

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

    I think that if we used (font-lock-mode -1) or some such higher-level
    command the problem would disappear since they are careful to save/restore
    the modification state of the buffer.

That would not do the job; it would fail to remove all the properties
which the current code removes.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Stefan Monnier]

>     I think that if we used (font-lock-mode -1) or some such higher-level
>     command the problem would disappear since they are careful to save/restore
>     the modification state of the buffer.
> 
> That would not do the job; it would fail to remove all the properties
> which the current code removes.

Can you give examples ?
Are these properties that the mode added to pre-existing text ?
Or did the mode add the text as well ?
I.e. do the properties belong to the mode or to the text ?


	Stefan

PS: And the current code fails to remove those properties if you didn't
    turn font-lock-mode ON, which doesn't make much sense either.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

    > That would not do the job; it would fail to remove all the properties
    > which the current code removes.

    Can you give examples ?

Modes that use font-lock-face are an example.  The current code
removes that property; just turning off font-lock-mode would not
remove it.

font-lock-face properties go in mode-specific places.

    PS: And the current code fails to remove those properties if you didn't
	turn font-lock-mode ON, which doesn't make much sense either.

Sorry, I don't understand.  Would you say that more clearly?

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Stefan Monnier]

>     > That would not do the job; it would fail to remove all the properties
>     > which the current code removes.
> 
>     Can you give examples ?
> 
> Modes that use font-lock-face are an example.  The current code
> removes that property; just turning off font-lock-mode would not
> remove it.
> 
> font-lock-face properties go in mode-specific places.

But do the properties belong to the text or to the mode.
I have the impression that most of those font-lock-face properties
are added together with the corresponding text and that they can't
be recovered just from the text.  I.e. erasing them might not
be the right thing to do since it loses information.

>     PS: And the current code fails to remove those properties if you didn't
> 	turn font-lock-mode ON, which doesn't make much sense either.
> 
> Sorry, I don't understand.  Would you say that more clearly?

The "erase the font-lock-face property when changing-mode" hook you
added is only added when you turn on font-lock-mode, so the
font-lock-face property will stay if you don't turn on font-lock-mode
before changing major-mode.

Examples:

	emacs -q --no-site-file
	M-x ibuffer
	M-x text-mode
	M-: (ibuffer-mode)
	M-x font-lock-mode

you see the font-locked text.  Now try this instead:

	emacs -q --no-site-file
	M-x ibuffer
	M-x font-lock-mode
	M-x text-mode
	M-: (ibuffer-mode)
	M-x font-lock-mode

now you don't see any fontification.
I actually think that the first behavior is more correct and
corresponds to what happens if you call (font-lock-mode -1) instead
of forcefully erasing the font-lock-face property.


	Stefan

PS: The elisp manual clearly says in the "major mode conventions"
node to "Define a command whose name ends in `-mode'" so ibuffer-mode
should be interactive.  I'd of course argue that it should use
`define-derived-mode' which would have made it interactive as a matter
of course without the programmer having to think about it.
As long as we don't use such a macro systematically, we'll have to live
with all those inconsistencies.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

    > font-lock-face properties go in mode-specific places.

    But do the properties belong to the text or to the mode.

I am not sure that question makes real sense, but the
font-lock-face property is not specific to any one mode.
The right font-lock-face properties for one mode are wrong
for some others.

    I have the impression that most of those font-lock-face properties
    are added together with the corresponding text and that they can't
    be recovered just from the text.  I.e. erasing them might not
    be the right thing to do since it loses information.

For Occur mode, this is correct.  Info mode automatically applies
the properties to the text.

With the current code, changing from Occur mode to Fundamental mode
and back to Occur mode would lose the font-lock-face properties.
That is a bug.

With the change you propose, changing from Occur mode to Fundamental
mode and then to Info mode would leave you with font-lock-face
properties left over from Occur mode.  That would be a bug.

Is there any way to make both cases work right?

    The "erase the font-lock-face property when changing-mode" hook you
    added is only added when you turn on font-lock-mode, so the
    font-lock-face property will stay if you don't turn on font-lock-mode
    before changing major-mode.

Now I understand.  Is there a method that fixes this too?

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Stefan Monnier]

>     > font-lock-face properties go in mode-specific places.
> 
>     But do the properties belong to the text or to the mode.
> 
> I am not sure that question makes real sense, but the
> font-lock-face property is not specific to any one mode.
> The right font-lock-face properties for one mode are wrong
> for some others.
> 
>     I have the impression that most of those font-lock-face properties
>     are added together with the corresponding text and that they can't
>     be recovered just from the text.  I.e. erasing them might not
>     be the right thing to do since it loses information.
> 
> For Occur mode, this is correct.  Info mode automatically applies
> the properties to the text.
> 
> With the current code, changing from Occur mode to Fundamental mode
> and back to Occur mode would lose the font-lock-face properties.
> That is a bug.
> 
> With the change you propose, changing from Occur mode to Fundamental
> mode and then to Info mode would leave you with font-lock-face
> properties left over from Occur mode.  That would be a bug.

I'm not sure how important this is, really.

> Is there any way to make both cases work right?

I think we can't make it work "automatically", but instead we need
to change `info.el' such that (font-lock-mode 1) or (font-lock-mode 0)
in info-mode begins by clearing the font-lock-face property.

We can maybe do it with something like

  (add-hook 'font-lock-mode
            (lambda ()
              (remove-face-properties (point-min) (point-max)
                                      '(font-lock-face))))

but that might be run too late.  So maybe we should add some
support in font-core.el to make it easier for a mode to tell
"erase font-lock-face property when font-lock-mode is changed".

Of course, another alternative is to make info.el use font-lock-keywords
(and the `face' property) rather than do all the looping&matching by hand.
That seems much better.

>     The "erase the font-lock-face property when changing-mode" hook you
>     added is only added when you turn on font-lock-mode, so the
>     font-lock-face property will stay if you don't turn on font-lock-mode
>     before changing major-mode.
> 
> Now I understand.  Is there a method that fixes this too?

First, I think that the change-major-mode-hook should
run (font-lock-mode -1) and that if it doesn't do the right thing,
then we should fix (font-lock-mode -1) rather than change
the change-major-mode-hook.


	Stefan

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Richard Stallman]

    > With the current code, changing from Occur mode to Fundamental mode
    > and back to Occur mode would lose the font-lock-face properties.
    > That is a bug.
    > 
    > With the change you propose, changing from Occur mode to Fundamental
    > mode and then to Info mode would leave you with font-lock-face
    > properties left over from Occur mode.  That would be a bug.

    I'm not sure how important this is, really.

It isn't worth a large amount of work.

    but that might be run too late.  So maybe we should add some
    support in font-core.el to make it easier for a mode to tell
    "erase font-lock-face property when font-lock-mode is changed".

The whole point of the font-lock-face property is that it can be set
up unconditionally, and is there regardless of whether Font-Lock mode
is enabled.  It would be wrong to remove these properties when turning
off Font-Lock mode.  Only changing the major mode is a reason to
remove them.

    First, I think that the change-major-mode-hook should
    run (font-lock-mode -1) and that if it doesn't do the right thing,
    then we should fix (font-lock-mode -1) rather than change
    the change-major-mode-hook.

For the reason given above, this is simply wrong.

Perhaps the modes that use font-lock-face should set up
change-major-mode-hook to clear it out.  That would solve the
inconsistency that you pointed out, that the font-lock-face properties
won't be cleared unless someone actually turned on font-lock-mode.
Everything will work right except cases like switching to Occur mode,
and those cases are not worth a lot of work to fix.

Re: problem with mh-e and 2002-08-20 change to font-core.el

[Stefan Monnier]

>     > With the current code, changing from Occur mode to Fundamental mode
>     > and back to Occur mode would lose the font-lock-face properties.
>     > That is a bug.
>     > 
>     > With the change you propose, changing from Occur mode to Fundamental
>     > mode and then to Info mode would leave you with font-lock-face
>     > properties left over from Occur mode.  That would be a bug.
> 
>     I'm not sure how important this is, really.
> 
> It isn't worth a large amount of work.
> 
>     but that might be run too late.  So maybe we should add some
>     support in font-core.el to make it easier for a mode to tell
>     "erase font-lock-face property when font-lock-mode is changed".
> 
> The whole point of the font-lock-face property is that it can be set
> up unconditionally, and is there regardless of whether Font-Lock mode
> is enabled.  It would be wrong to remove these properties when turning
> off Font-Lock mode.  Only changing the major mode is a reason to
> remove them.

Good point.

>     First, I think that the change-major-mode-hook should
>     run (font-lock-mode -1) and that if it doesn't do the right thing,
>     then we should fix (font-lock-mode -1) rather than change
>     the change-major-mode-hook.
> 
> For the reason given above, this is simply wrong.
> 
> Perhaps the modes that use font-lock-face should set up
> change-major-mode-hook to clear it out.  That would solve the

Agreed.


	Stefan