buffer is modified after commit

buffer is modified after commit

[Juanma Barranquero]

I'm seeing a maddening heinsenbug where, after committing to CVS (via
vc-toggle-read-only) a file visited in a non-modified buffer, the
buffer is marked as modified.

I'm quite sure it is not an Emacs bug, but something in my setup (it
does not happen with "emacs -q"; it does happen in my two main
computers without -q).

However, it started very recently (possibly after the unicode merge,
though I'm not really sure), and so far it has resisted any attempt to
locate it via bisection of .emacs. All I know is that it seems to be
related to diff switches, i.e.,

   diff-switches
   vc-cvs-diff-switches
   vc-svn-diff-swtiches (I don't know why, because there aren't .svn
dirs in the tree)
   the setting of `diff' in ~/.cvsrc

Has someone seen something like that, or can anyone shed any light on it?

             Juanma

Re: buffer is modified after commit

[Juanma Barranquero]

Well, after digging in my .emacs for a while, and several false
starts, I've finally been able to isolate the "buffer is modified
after commit" bug that was driving me crazy. Or, at least, I can now
repeat it at will.

Please try with this simple .emacs:

;;;;; .emacs ;;;;;
(global-highlight-changes 1)
;;;;;;;;;;;;;;;;;;

and then commit a file (I see the error with CVS and Mercurial
backends, I suppose it is generic but I haven't tested any other).
After the commit, the file is marked as modified.

The thing is, it does not happen in EMACS_22_BASE, but hilit-chg.el in
the branch is identical to the trunk version, so I suspect the bug is
somehow triggered by the recent VC changes.

Could someone with VC experience please take a look at it?

(BTW, the heisenbug nature of the bug was caused by desktop.el kindly
saving the highlight-change-mode value without me being aware of
it...)

Thanks,

             Juanma

Re: buffer is modified after commit

[martin rudalics]

 > Please try with this simple .emacs:
 >
 > ;;;;; .emacs ;;;;;
 > (global-highlight-changes 1)
 > ;;;;;;;;;;;;;;;;;;
 >
 > and then commit a file (I see the error with CVS and Mercurial
 > backends, I suppose it is generic but I haven't tested any other).
 > After the commit, the file is marked as modified.

I suppose it's due to the way Highlight Changes mode adds and removes
text properties.  Could you save the values of `buffer-modified-tick'
and `buffer-chars-modified-tick' before committing and compare the saved
values with the actual ones after the commit?

Re: buffer is modified after commit

[Stefan Monnier]

> Well, after digging in my .emacs for a while, and several false
> starts, I've finally been able to isolate the "buffer is modified
> after commit" bug that was driving me crazy.  Or, at least, I can now
> repeat it at will.

> ;;;;; .emacs ;;;;;
> (global-highlight-changes 1)
> ;;;;;;;;;;;;;;;;;;

If you look at hilit-chg.el you'll see that it's messing up the
buffer-modified-p on a regular basis.

1 - buffer-modified-p is properly saved&restored in
    highlight-changes-rotate-faces and in highlight-markup-buffers.
    That's the good part.
2 - it's not properly saved&restored anywhere else, despite uses of things
    like remove-text-properties.  That's the source of your trouble.
3 - buffer-modified-p is incorrectly forcefully set to nil by the
    special undo entries added in highlight-changes-rotate-faces.
    That's correct if the buffer was not modified before undoing
    this command, but is dangerous otherwise (will mark your buffer as
    unmodified even though you haven't saved the changes).


-- Stefan

Re: buffer is modified after commit

[martin rudalics]

 > 3 - buffer-modified-p is incorrectly forcefully set to nil by the
 >     special undo entries added in highlight-changes-rotate-faces.
 >     That's correct if the buffer was not modified before undoing
 >     this command, but is dangerous otherwise (will mark your buffer as
 >     unmodified even though you haven't saved the changes).

Yes, that part is tricky and should be solved by a generic primitive
rather than my ad-hoc fix.  I thought that when the buffer is unmodified
at the time I do "this command", doing/undoing "this command" and
setting the modified status to nil should not cause any harm.  Can you
provide a counterexample?

Re: buffer is modified after commit

[Stefan Monnier]

>> 3 - buffer-modified-p is incorrectly forcefully set to nil by the
>> special undo entries added in highlight-changes-rotate-faces.
>> That's correct if the buffer was not modified before undoing
>> this command, but is dangerous otherwise (will mark your buffer as
>> unmodified even though you haven't saved the changes).

> Yes, that part is tricky and should be solved by a generic primitive
> rather than my ad-hoc fix.  I thought that when the buffer is unmodified
> at the time I do "this command", doing/undoing "this command" and
> setting the modified status to nil should not cause any harm.  Can you
> provide a counterexample?

edit
save
rotate
edit
save
undo undo undo...

By the way, I've installed a change on the 22 branch which should fix
the buffer-modified-p problem (and removes this number 3 hack).


        Stefan

Re: buffer is modified after commit

[Juanma Barranquero]

On Feb 13, 2008 11:01 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> By the way, I've installed a change on the 22 branch which should fix
> the buffer-modified-p problem (and removes this number 3 hack).

I've tried it on the trunk, and the problem seems indeed to be fixed.

Thanks.

             Juanma

Re: buffer is modified after commit

[martin rudalics]

 > edit
 > save
 > rotate
 > edit
 > save
 > undo undo undo...

Maybe define

(defun restore-buffer-unmodified-unless-saved (arg)
   (when (or (and (not (buffer-file-name)) (not arg))
	    (and (file-exists-p (buffer-file-name))
		 (equal arg (nth 5 (file-attributes (buffer-file-name))))))
     (restore-buffer-modified nil)))

and add

(setq buffer-undo-list
       (cons '(apply restore-buffer-unmodified-unless-saved
		    (when (and (buffer-file-name)
			       (file-exists-p (buffer-file-name)))
		      (nth 5 (file-attributes (buffer-file-name)))))
	    buffer-undo-list))

to `highlight-changes-rotate-faces', but that would constitute yet
another ugly hack.

What's really needed is a new entry type for `buffer-undo-list' like the
(nil PROPERTY VALUE BEG . END) one but when applied not causing a change
in the buffer-modified status.

 > By the way, I've installed a change on the 22 branch which should fix
 > the buffer-modified-p problem (and removes this number 3 hack).

Thanks.

Re: buffer is modified after commit

[Stefan Monnier]

>> edit
>> save
>> rotate
>> edit
>> save
>> undo undo undo...

> Maybe define

> (defun restore-buffer-unmodified-unless-saved (arg)
>   (when (or (and (not (buffer-file-name)) (not arg))
> 	    (and (file-exists-p (buffer-file-name))
> 		 (equal arg (nth 5 (file-attributes (buffer-file-name))))))
>     (restore-buffer-modified nil)))

> and add

> (setq buffer-undo-list
>       (cons '(apply restore-buffer-unmodified-unless-saved
> 		    (when (and (buffer-file-name)
> 			       (file-exists-p (buffer-file-name)))
> 		      (nth 5 (file-attributes (buffer-file-name)))))
> 	    buffer-undo-list))

> to `highlight-changes-rotate-faces', but that would constitute yet
> another ugly hack.

> What's really needed is a new entry type for `buffer-undo-list' like the
> (nil PROPERTY VALUE BEG . END) one but when applied not causing a change
> in the buffer-modified status.

There's no such need.  The `apply' entry is all-powerful.
What hilit-chg needs to do is something like:

   (defun hilit-chg-undo-nomodify (undo-list)
     (let ((modified (buffer-modified-p)))
       (while undo-list
         (setq undo-list (primitive-undo 1 undo-list)))
       (unless modified (restore-buffer-modified-p nil))))

and then use something like:

   (push
    (let* ((buffer-undo-list nil))
      ...do your thing...
      `(apply 'hilit-chg-undo-nomodify ,buffer-undo-list))
    buffer-undo-list)

Of course, another option is to define a reverse function from
(unrotate) and put that one on the undo-list (that's what I (try to) do
with diff-reverse-direction, diff-context->unified, and
diff-unified->context).


        Stefan

Re: buffer is modified after commit

[martin rudalics]

 >    (push
 >     (let* ((buffer-undo-list nil))
 >       ...do your thing...
 >       `(apply 'hilit-chg-undo-nomodify ,buffer-undo-list))
 >     buffer-undo-list)

It would be very, very helpful if you turned this into a macro.  IMHO
subr.el should provide two macros: A standard one allowing to change
text properties on-the-fly and this one to additionally specify undo
information.  If you look at the current emacs sources you can find at
least ten ways how people decided to change text properties on-the-fly
and some of these might have to be undoable as well.  I also wonder how
often people have used overlays only to escape the problem of handling
the modified state of buffers (in one case the author apparently saved
the state even around an overlay modification).