bug#13949: 24.4.1; `fill-paragraph' should not always put the buffer as modified

["Óscar Fuentes" <ofv@wanadoo.es>]

John Wiegley <jwiegley@gmail.com> writes:

> So we must ask ourselves: What will fixing this issue actually solve? We'd no
> longer modify timestamps when unnecessary, and the user wouldn't feel
> compelled to save at times when it is not needed. That is all I can think of.
> Therefore, this bug is truly a wishlist item.
>
> I've noticed over the past couple of decades that M-q always sets my modified
> flag. It never once occurred to me that this should be considered a problem.

Emacs thinks that the buffer is modified when actually it isn't, and
gives this false information to the user and to itself. So we can't no
longer rely on the modeline indicator to know if the file was modified.

Some features and packages (M-x compile, magit) ask the user when they
are invoked and there are buffers with unsaved changes. Saving a buffer
that doesn't change the file's contents (it just updates the file's
timestamp) may cause undesirable effects, like triggering a lengthy
build of a project. And so on. So it is not true that this "wishlist"
issue has no serious effects. Emacs is well below its usual level of
cleverness here.

Computing hashes of the paragraph (or the whole buffer, if you wish)
before and after the operation and comparing them was suggested. Luckily
it is not complex at all. We already have `secure-hash' which can
operate on whole buffers or ranges. I attach a quick and dirty proof of
concept for fill-paragraph, which should be useful for evaluating
worst-case performance impact. This approach is not enough, as there are
other functions (such as lisp-fill-paragraph) that shows the same
problem. The low level functions which are used by *fill-paragraph are
the ones that should be patched.

To Jaakov: I agree with you that this is a bug, and not a minor one at
that. However, the severity associated to this or any other report is
mostly irrelevant. Solving the problem depends on the existence of
someone who is willing to fix the issue. Almost all contributors here
work on what they decide to work on, and utterly ignore those labels.


diff --git a/lisp/textmodes/fill.el b/lisp/textmodes/fill.el
index 100e2a2..9e1f430 100644
--- a/lisp/textmodes/fill.el
+++ b/lisp/textmodes/fill.el
@@ -804,6 +804,7 @@ fill-paragraph
   (interactive (progn
 		 (barf-if-buffer-read-only)
 		 (list (if current-prefix-arg 'full) t)))
+  (setq h (if (buffer-modified-p) "" (secure-hash 'md5 (current-buffer))))
   (or
    ;; 1. Fill the region if it is active when called interactively.
    (and region transient-mark-mode mark-active
@@ -862,7 +863,10 @@ fill-paragraph
                      ;; fill-region.
                      (fill-region beg end justify)
                    (fill-region-as-paragraph beg end justify))))))
-     fill-pfx)))
+     fill-pfx))
+  (when (and (not (string= h ""))
+             (string= h (secure-hash 'md5 (current-buffer))))
+    (set-buffer-modified-p nil)))
 
 (declare-function comment-search-forward "newcomment" (limit &optional noerror))
 (declare-function comment-string-strip "newcomment" (str beforep afterp))