Subject: Re: bug#72341: VC: CVS template lines not stripped when committing

From: Christoph Badura <[email-masked]>
Date: Tue, 18 Feb 2025 00:54:26 +0100

Here is the updated patch.  I incorporated all the feedback and requested
changes.  I've also found some other nits on my own.

I also looked more carefully at the CVS code and discovered that it only
looks for lines beginning with "CVS:", i.e., it doesn't care about a
following blank.  It does this in order to deal with editor that strip
trailing whitespace.

Changes since the last patch:
- don't activate log-edit-done-strip-cvs-lines by default just yet
- supply missing arg 'interactive to called-interactively-p
- use (case-fold-search nil) instead of (search-upper-case nil)
- mention log-edit-done-strip-cvs-lines in etc/NEWS
- explain why log-edit-done-strip-cvs-lines only looks for "^CVS:"
- doc strings for the log-edit-done-strip-cvs-lines tests
- factor out common code in log-edit-done-strip-cvs-lines tests
- 2 more tests for exactly the same behaviour as CVS

I've just noticed that, e.g. the lines following 
the "hook :options" in the log-edit-done-hook
defcustom, use TAB for indentation while indent-tabs-mode is turned off in
the top-level .dir-locals.el.  Do you care for consistency with the old
code?  This affects only a very small number of lines.

Sorry for this all taking so long.  And thanks for the feedback and the
patience.

--chris
>From c7c0d1c47ce283a356e381dcc20e22c3e3d4d657 Mon Sep 17 00:00:00 2001
From: Christoph Badura <[email-masked]>
Date: Sun, 26 Jan 2025 22:48:11 +0100
Subject: [PATCH] VC: add hook to strip CVS template lines when committing

Add a hook function to strip all lines beginning with "CVS:" from the
commit message as CVS does.  Do this only if 'log-edit-vc-backend' is
'CVS'.  (Bug#72341)
* lisp/vc/log-edit.el (log-edit-done-strip-cvs-lines)
Add a hook function to strip the lines starting with "CVS:".
* test/lisp/vc/log-edit-tests.el (log-edit-done-strip-cvs-lines-helper,
log-edit-done-strip-cvs-lines-cvs, log-edit-done-strip-cvs-lines-non-cvs,
log-edit-done-strip-cvs-lines-only-cvs-colon-blank,
log-edit-done-strip-cvs-lines-only-cvs-colon):
Add test cases for 'log-edit-done-strip-cvs-lines'.
* etc/NEWS:
Mention log-edit-done-strip-cvs-lines.
---
 etc/NEWS                       |  8 +++++
 lisp/vc/log-edit.el            | 18 ++++++++++-
 test/lisp/vc/log-edit-tests.el | 58 ++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 22cf8c636c2..c6b4aa6e7e2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -343,6 +343,14 @@ modal editing packages.
 
 * Changes in Specialized Modes and Packages in Emacs 31.1
 
+---
+** VC mode
+*** New function 'log-edit-done-strip-cvs-lines'.
+This function strips all lines beginning with "CVS:" from the buffer.
+It is intended to be added to the 'log-edit-done-hook' so that
+'vc-cvs-checkin' behaves like invoking "cvs commit [files...]" from the
+command line.
+
 ---
 ** ASM mode
 *** 'asm-mode-set-comment-hook' is obsolete.
diff --git a/lisp/vc/log-edit.el b/lisp/vc/log-edit.el
index e23e7414a18..1b0fa0d7193 100644
--- a/lisp/vc/log-edit.el
+++ b/lisp/vc/log-edit.el
@@ -210,7 +210,8 @@ log-edit-done-hook
 can be obtained from `log-edit-files'."
   :group 'log-edit
   :type '(hook :options (log-edit-set-common-indentation
-			 log-edit-add-to-changelog)))
+			 log-edit-add-to-changelog
+                         log-edit-done-strip-cvs-lines)))
 
 (defcustom log-edit-strip-single-file-name nil
   "If non-nil, remove file name from single-file log entries."
@@ -926,6 +927,21 @@ log-edit-insert-cvs-template
       (goto-char (point-max))
       (insert-file-contents "CVS/Template"))))
 
+(defun log-edit-done-strip-cvs-lines ()
+  "Strip lines starting with \"CVS:\" from commit log message.
+When not called interactively do this only when the VC backend is CVS.
+This mimicks what CVS does when invoked as 'cvs commit [files...]'"
+  (interactive)
+  (when (or (called-interactively-p 'interactive)
+            (eq log-edit-vc-backend 'CVS))
+    (let ((case-fold-search nil))
+      (goto-char (point-min))
+      ;; NB: While CVS defines CVSEDITPREFIX as "CVS: " it actually
+      ;; checks only the first four characters of af a line, i.e. "CVS:"
+      ;; do deal with editors that strip trailing whitespace.
+      ;; c.f. src/cvs.h and src/logmsg.c:do_editor()
+      (flush-lines "^CVS:"))))
+
 (defun log-edit-insert-cvs-rcstemplate ()
   "Insert the RCS commit log template from the CVS repository.
 This contacts the repository to get the rcstemplate file and
diff --git a/test/lisp/vc/log-edit-tests.el b/test/lisp/vc/log-edit-tests.el
index 005c336a3b6..515abe5d598 100644
--- a/test/lisp/vc/log-edit-tests.el
+++ b/test/lisp/vc/log-edit-tests.el
@@ -360,4 +360,62 @@ log-edit-fill-entry-no-defun-list-wrapping
       (let ((fill-column 64)) (log-edit-fill-entry))
       (should (equal (buffer-string) wanted)))))
 
+(defun log-edit-done-strip-cvs-lines-helper (initial-text wanted vc-backend)
+  "Test that running log-edit-done-strip-cvs-lines as a
+log-edit-done-hook produces the WANTED
+string when run on INITIAL-TEXT."
+  (with-temp-buffer
+      (let ((log-edit-done-hook 'log-edit-done-strip-cvs-lines)
+            (log-edit-vc-backend vc-backend))
+        (setq-local log-edit-callback #'(lambda () (interactive) nil))
+        (insert initial-text)
+        (log-edit-done)
+        (should (equal (buffer-string) wanted)))))
+
+(ert-deftest log-edit-done-strip-cvs-lines-cvs ()
+  "Strip lines beginning with \"CVS:\" when using CVS as VC backend."
+  ;; This test verifies that lines beginning with "CVS:" are stripped
+  ;; from the commit message when log-edit-vc-backend is CVS.
+  (let (string wanted)
+    (setq string "summary line
+first line
+CVS: Please evaluate your changes and consider the following.
+CVS: Abort checkin if you answer no.
+"
+          wanted "summary line
+first line
+")
+    (log-edit-done-strip-cvs-lines-helper string wanted 'CVS)))
+
+(ert-deftest log-edit-done-strip-cvs-lines-non-cvs ()
+  "Do not strip lines beginning with \"CVS:\" when not using CVS as VC backend."
+  ;; This test verifies that lines beginning with "CVS:" are not stripped
+  ;; from the commit message when log-edit-cvs-backend isn't CVS.
+  (let (string)
+    (setq string "summary line
+first line
+CVS: Please evaluate your changes and consider the following.
+CVS: Abort checkin if you answer no.
+")
+  (log-edit-done-strip-cvs-lines-helper string string nil)))
+
+(ert-deftest log-edit-done-strip-cvs-lines-only-cvs-colon-blank ()
+  "Strip lines that contain solely \"CVS: \" when using CVS as VC backend."
+  ;; This test verifies that lines consisting solely of "CVS: " are stripped
+  ;; from the commit message.
+  (let (string wanted)
+    (setq string "CVS: \n"
+          wanted "")
+    (log-edit-done-strip-cvs-lines-helper string wanted 'CVS)))
+
+(ert-deftest log-edit-done-strip-cvs-lines-only-cvs-colon ()
+  "Strip lines that contain solely \"CVS:\" when using CVS as VC backend."
+  ;; This test verifies that lines consisting only of "CVS:" (no blank
+  ;; after the colon) are stripped from the commit message.
+  ;; CVS does this to accomodate editors that delete trailing whitespace.
+  (let (string wanted)
+    (setq string "CVS:\n"
+          wanted "")
+    (log-edit-done-strip-cvs-lines-helper string wanted 'CVS)))
+
 ;;; log-edit-tests.el ends here
-- 
2.47.1