Improved ChangeLog generation for vc log (Bug#16301)

* lisp/vc/diff-mode.el (diff-find-source-location): Fix docstring.

* lisp/vc/add-log.el (change-log-unindented-file-names-re)
(change-log-read-entries, change-log-read-defuns)
(change-log-insert-entries):
* lisp/vc/diff-mode.el (diff-add-log-current-defuns):
* lisp/vc/log-edit.el (log-edit--insert-filled-defuns)
(log-edit-fill-entry): New functions.
(log-edit-mode): Set `log-edit-fill-entry' as
`fill-paragraph-function'.
(log-edit-generate-changelog-from-diff): New command.
(log-edit-mode-map): Bind it to C-c C-w.
* doc/emacs/maintaining.texi (Types of Log File, Log Buffer):
* CONTRIBUTE: Document it.
* etc/NEWS: Announce it.
* test/lisp/vc/log-edit-tests.el (log-edit-fill-entry)
(log-edit-fill-entry-joining): New tests.

3 files changed, 241 insertions, 4 deletions

diff --git a/lisp/vc/add-log.el b/lisp/vc/add-log.el
index f9efd44c5c7..47a68167fb7 100644
--- a/lisp/vc/add-log.el
+++ b/lisp/vc/add-log.el
@@ -36,6 +36,8 @@
 
 ;;; Code:
 
+(eval-when-compile (require 'cl-lib))
+
 (defgroup change-log nil
   "Change log maintenance."
   :group 'tools
@@ -309,6 +311,43 @@ a case simply use the directory containing the changed file."
 	  (re-search-forward change-log-file-names-re nil t)
 	  (match-string-no-properties 2))))))
 
+(defconst change-log-unindented-file-names-re "^[*] \\([^ ,:([\n]+\\)")
+
+(defun change-log-read-entries (&optional end)
+  "Read ChangeLog entries at point until END.
+Move point to the end of entries that were read.  Return a list
+in the same form as `diff-add-log-current-defuns'."
+  (cl-loop while (and (or (not end) (< (point) end))
+                      (looking-at change-log-unindented-file-names-re))
+           do (goto-char (match-end 0))
+           collect (cons (match-string-no-properties 1)
+                         (change-log-read-defuns end))))
+
+(defvar change-log-tag-re) ; add-log.el
+(defun change-log-read-defuns (&optional end)
+  "Read ChangeLog formatted function names at point until END.
+Move point to the end of names read and return the function names
+as a list of strings."
+  (cl-loop while (and (skip-chars-forward ":\n[:blank:]" end)
+                      (or (not end) (< (point) end))
+                      (looking-at change-log-tag-re))
+           do (goto-char (match-end 0))
+           nconc (split-string (match-string-no-properties 1)
+                               ",[[:blank:]]*" t)
+           finally do (skip-chars-backward "\n[:blank:]")))
+
+(defun change-log-insert-entries (changelogs)
+  "Format and insert CHANGELOGS into current buffer.
+CHANGELOGS is a list in the form returned by
+`diff-add-log-current-defuns'."
+  (cl-loop for (file . defuns) in changelogs do
+           (insert "* " file)
+           (if (not defuns)
+               (insert ":\n")
+             (insert " ")
+             (cl-loop for def in defuns
+                      do (insert "(" def "):\n")))))
+
 (defun change-log-find-file ()
   "Visit the file for the change under point."
   (interactive)
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 0d5dc0e1c0c..81662cafeda 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -54,6 +54,7 @@
 
 ;;; Code:
 (eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'subr-x))
 
 (autoload 'vc-find-revision "vc")
 (autoload 'vc-find-revision-no-save "vc")
@@ -1773,15 +1774,22 @@ Whitespace differences are ignored."
 (defsubst diff-xor (a b) (if a (if (not b) a) b))
 
 (defun diff-find-source-location (&optional other-file reverse noprompt)
-  "Find out (BUF LINE-OFFSET POS SRC DST SWITCHED).
+  "Find current diff location within the source file.
+OTHER-FILE, if non-nil, means to look at the diff's name and line
+  numbers for the old file.  Furthermore, use `diff-vc-revisions'
+  if it's available.  If `diff-jump-to-old-file' is non-nil, the
+  sense of this parameter is reversed.  If the prefix argument is
+  8 or more, `diff-jump-to-old-file' is set to OTHER-FILE.
+REVERSE, if non-nil, switches the sense of SRC and DST (see below).
+NOPROMPT, if non-nil, means not to prompt the user.
+Return a list (BUF LINE-OFFSET (BEG . END) SRC DST SWITCHED).
 BUF is the buffer corresponding to the source file.
 LINE-OFFSET is the offset between the expected and actual positions
   of the text of the hunk or nil if the text was not found.
-POS is a pair (BEG . END) indicating the position of the text in the buffer.
+\(BEG . END) is a pair indicating the position of the text in the buffer.
 SRC and DST are the two variants of text as returned by `diff-hunk-text'.
   SRC is the variant that was found in the buffer.
-SWITCHED is non-nil if the patch is already applied.
-NOPROMPT, if non-nil, means not to prompt the user."
+SWITCHED is non-nil if the patch is already applied."
   (save-excursion
     (let* ((other (diff-xor other-file diff-jump-to-old-file))
 	   (char-offset (- (point) (diff-beginning-of-hunk t)))
@@ -2210,6 +2218,121 @@ Call FUN with two args (BEG and END) for each hunk."
   (let ((inhibit-read-only t))
     (undo arg)))
 
+(defun diff-add-log-current-defuns ()
+  "Return an alist of defun names for the current diff.
+The elements of the alist are of the form (FILE . (DEFUN...)),
+where DEFUN... is a list of function names found in FILE."
+  (save-excursion
+    (goto-char (point-min))
+    (let ((defuns nil)
+          (hunk-end nil)
+          (hunk-mismatch-files nil)
+          (make-defun-context-follower
+           (lambda (goline)
+             (let ((eodefun nil)
+                   (defname nil))
+               (list
+                (lambda () ;; Check for end of current defun.
+                  (when (and eodefun
+                             (funcall goline)
+                             (>= (point) eodefun))
+                    (setq defname nil)
+                    (setq eodefun nil)))
+                (lambda (&optional get-current) ;; Check for new defun.
+                  (if get-current
+                      defname
+                    (when-let* ((def (and (not eodefun)
+                                          (funcall goline)
+                                          (add-log-current-defun)))
+                                (eof (save-excursion (end-of-defun) (point))))
+                      (setq eodefun eof)
+                      (setq defname def)))))))))
+      (while
+          ;; Might need to skip over file headers between diff
+          ;; hunks (e.g., "diff --git ..." etc).
+          (re-search-forward diff-hunk-header-re nil t)
+        (setq hunk-end (save-excursion (diff-end-of-hunk)))
+        (pcase-let* ((filename (substring-no-properties (diff-find-file-name)))
+                     (=lines 0)
+                     (+lines 0)
+                     (-lines 0)
+                     (`(,buf ,line-offset (,beg . ,_end)
+                             (,old-text . ,_old-offset)
+                             (,new-text . ,_new-offset)
+                             ,applied)
+                      ;; Try to use the vc integration of
+                      ;; `diff-find-source-location', unless it
+                      ;; would look for non-existent files like
+                      ;; /dev/null.
+                      (diff-find-source-location
+                       (not (equal "/dev/null"
+                                   (car (diff-hunk-file-names t))))))
+                     (other-buf nil)
+                     (goto-otherbuf
+                      ;; If APPLIED, we have NEW-TEXT in BUF, so we
+                      ;; need to a buffer with OLD-TEXT to follow
+                      ;; -lines.
+                      (lambda ()
+                        (if other-buf (set-buffer other-buf)
+                          (set-buffer (generate-new-buffer " *diff-other-text*"))
+                          (insert (if applied old-text new-text))
+                          (funcall (buffer-local-value 'major-mode buf))
+                          (setq other-buf (current-buffer)))
+                        (goto-char (point-min))
+                        (forward-line (+ =lines -1
+                                         (if applied -lines +lines)))))
+                     (gotobuf (lambda ()
+                                (set-buffer buf)
+                                (goto-char beg)
+                                (forward-line (+ =lines -1
+                                                 (if applied +lines -lines)))))
+                     (`(,=ck-eodefun ,=ck-defun)
+                      (funcall make-defun-context-follower gotobuf))
+                     (`(,-ck-eodefun ,-ck-defun)
+                      (funcall make-defun-context-follower
+                               (if applied goto-otherbuf gotobuf)))
+                     (`(,+ck-eodefun ,+ck-defun)
+                      (funcall make-defun-context-follower
+                               (if applied gotobuf goto-otherbuf))))
+          (unless (eql line-offset 0)
+            (cl-pushnew filename hunk-mismatch-files :test #'equal))
+          ;; Some modes always return nil for `add-log-current-defun',
+          ;; make sure at least the filename is included.
+          (unless (assoc filename defuns)
+            (push (cons filename nil) defuns))
+          (unwind-protect
+              (while (progn (forward-line)
+                            (< (point) hunk-end))
+                (let ((patch-char (char-after)))
+                  (pcase patch-char
+                    (?+ (cl-incf +lines))
+                    (?- (cl-incf -lines))
+                    (?\s (cl-incf =lines)))
+                  (save-current-buffer
+                    (funcall =ck-eodefun)
+                    (funcall +ck-eodefun)
+                    (funcall -ck-eodefun)
+                    (when-let* ((def (cond
+                                      ((eq patch-char ?\s)
+                                       ;; Just updating context defun.
+                                       (ignore (funcall =ck-defun)))
+                                      ;; + or - in existing defun.
+                                      ((funcall =ck-defun t))
+                                      ;; Check added or removed defun.
+                                      (t (funcall (if (eq ?+ patch-char)
+                                                      +ck-defun -ck-defun))))))
+                      (cl-pushnew def (alist-get filename defuns
+                                                 nil nil #'equal)
+                                  :test #'equal)))))
+            (when (buffer-live-p other-buf)
+              (kill-buffer other-buf)))))
+      (when hunk-mismatch-files
+        (message "Diff didn't match for %s."
+                 (mapconcat #'identity hunk-mismatch-files ", ")))
+      (dolist (file-defuns defuns)
+        (cl-callf nreverse (cdr file-defuns)))
+      (nreverse defuns))))
+
 (defun diff-add-change-log-entries-other-window ()
   "Iterate through the current diff and create ChangeLog entries.
 I.e. like `add-change-log-entry-other-window' but applied to all hunks."
diff --git a/lisp/vc/log-edit.el b/lisp/vc/log-edit.el
index 91e18c1ec5c..8d47d66ac38 100644
--- a/lisp/vc/log-edit.el
+++ b/lisp/vc/log-edit.el
@@ -54,6 +54,7 @@
 (easy-mmode-defmap log-edit-mode-map
   '(("\C-c\C-c" . log-edit-done)
     ("\C-c\C-a" . log-edit-insert-changelog)
+    ("\C-c\C-w" . log-edit-generate-changelog-from-diff)
     ("\C-c\C-d" . log-edit-show-diff)
     ("\C-c\C-f" . log-edit-show-files)
     ("\C-c\C-k" . log-edit-kill-buffer)
@@ -488,10 +489,63 @@ commands (under C-x v for VC, for example).
   (set (make-local-variable 'font-lock-defaults)
        '(log-edit-font-lock-keywords t))
   (setq-local jit-lock-contextually t)  ;For the "first line is summary".
+  (setq-local fill-paragraph-function #'log-edit-fill-entry)
   (make-local-variable 'log-edit-comment-ring-index)
   (add-hook 'kill-buffer-hook 'log-edit-remember-comment nil t)
   (hack-dir-local-variables-non-file-buffer))
 
+(defun log-edit--insert-filled-defuns (func-names)
+  "Insert FUNC-NAMES, following ChangeLog formatting."
+  (if (not func-names)
+      (insert ":")
+    (unless (or (memq (char-before) '(?\n ?\s))
+                (> (current-column) fill-column))
+      (insert " "))
+    (cl-loop for first-fun = t then nil
+             for def in func-names do
+             (when (> (+ (current-column) (string-width def)) fill-column)
+               (unless first-fun
+                 (insert ")"))
+               (insert "\n"))
+             (insert (if (memq (char-before) '(?\n ?\s))
+                         "(" ", ")
+                     def))
+    (insert "):")))
+
+(defun log-edit-fill-entry (&optional justify)
+  "Like \\[fill-paragraph], but handle ChangeLog entries.
+Consecutive function entries without prose (i.e., lines of the
+form \"(FUNCTION):\") will be combined into \"(FUNC1, FUNC2):\"
+according to `fill-column'."
+  (save-excursion
+    (pcase-let ((`(,beg ,end) (log-edit-changelog-paragraph)))
+      (if (= beg end)
+          ;; Not a ChangeLog entry, fill as normal.
+          nil
+        (cl-callf copy-marker end)
+        (goto-char beg)
+        (cl-loop
+         for defuns-beg =
+         (and (< beg end)
+              (re-search-forward
+               (concat "\\(?1:" change-log-unindented-file-names-re
+                       "\\)\\|^\\(?1:\\)(")
+               end t)
+              (copy-marker (match-end 1)))
+         ;; Fill prose between log entries.
+         do (let ((fill-indent-according-to-mode t)
+                  (end (if defuns-beg (match-beginning 0) end))
+                  (beg (progn (goto-char beg) (line-beginning-position))))
+              (when (<= (line-end-position) end)
+                (fill-region beg end justify)))
+         while defuns-beg
+         for defuns = (progn (goto-char defuns-beg)
+                             (change-log-read-defuns end))
+         do (progn (delete-region defuns-beg (point))
+                   (log-edit--insert-filled-defuns defuns)
+                   (setq beg (point))))
+        t))))
+
 (defun log-edit-hide-buf (&optional buf where)
   (when (setq buf (get-buffer (or buf log-edit-files-buf)))
     ;; FIXME: Should use something like `quit-windows-on' here, but
@@ -726,6 +780,27 @@ to build the Fixes: header.")
       (replace-match (concat " " value) t t nil 1)
     (insert field ": " value "\n" (if (looking-at "\n") "" "\n"))))
 
+(declare-function diff-add-log-current-defuns "diff-mode" ())
+
+(defun log-edit-generate-changelog-from-diff ()
+  "Insert a log message by looking at the current diff.
+This command will generate a ChangeLog entries listing the
+functions.  You can then add a description where needed, and use
+\\[fill-paragraph] to join consecutive function names."
+  (interactive)
+  (let* ((diff-buf nil)
+         ;; Unfortunately, `log-edit-show-diff' doesn't have a NO-SHOW
+         ;; option, so we try to work around it via display-buffer
+         ;; machinery.
+         (display-buffer-overriding-action
+          `(,(lambda (buf alist)
+               (setq diff-buf buf)
+               (display-buffer-no-window buf alist))
+            . ((allow-no-window . t)))))
+    (change-log-insert-entries
+     (with-current-buffer (progn (log-edit-show-diff) diff-buf)
+       (diff-add-log-current-defuns)))))
+
 (defun log-edit-insert-changelog (&optional use-first)
   "Insert a log message by looking at the ChangeLog.
 The idea is to write your ChangeLog entries first, and then use this