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

From: Christoph Badura <[email-masked]>
Date: Mon, 27 Jan 2025 00:08:26 +0100

Sorry for the long delay.

I guess I better start with a short summary to spare you the trouble of
reading through the previous messages relating to this bug.

When "cvs commit [file...]" is invoked from the command line, cvs prepares
a temp file with a template of a commit message and invokes an editor on
it. The template commit message, by default, is like this:

CVS: ----------------------------------------------------------------------
CVS: Enter Log.  Lines beginning with `CVS:' are removed automatically
CVS: 
CVS: Committing in .
CVS: 
CVS: Modified Files:
CVS: 	test.txt 
CVS: ----------------------------------------------------------------------

After saving the edited commit message and exiting the editor, cvs strips
lines beginning with 'CVS:' from the temp file.  NB. this is the only
documentation for this feature.  The CVS manual doesn't mention this.

When committing with "cvs commit -m 'Commit message.' [files...]" this
stripping is *not* performed.

The commit message template in the temp file is augmented with the
contents of the file 'CVS/Template', if it exists or a template received
from the CVS server..

I work on NetBSD where we make use of such a template.

'vc-cvs-checkin' emulates the first part of this (inserting the template
from the server/'CVS/Template file.  However, it does not strip the lines
beginning with "CVS:" and uses the "cvs commit -m 'Commit message.'" method
to do the actual commit.  That causes the "CVS:"-lines to end up in the
actual commit.

I have drafted a change that adds a function to the 'log-edit-done-hook'
that strips lines beginning with "CVS:" from the commit message if the
'log-edit-vc-backend' is "'CVS".

Because this hook function does its work only when the
'log-edit-vc-backend' is CVS and since this just makes 'vc-cvs-checkin'
behave like invking "cvs commit [files...]" from the command line I
consider this a change low risk and don't think that it would surprise
existing users.  Therefore I think it is appropriate to enable this hook
by default.

It would be nice if this would end up in emacs-30.1.

I'd appreciate any review and guidance what is missing to make this
committable.

I guess I need to add an entry to the NEWS file.

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

Strip all lines beginning with "CVS: " from the commit
message as CVS does (Bug#72341).  Do this only if 'log-edit-vc-backend'
is 'CVS'.
* lisp/vc/log-edit.el (log-edit-done-strip-cvs-lines, log-edit-done-hook):
Add a hook function to strip the lines starting with "CVS: " and make it
to the 'log-edit-done-hook' by default.
* test/lisp/vc/log-edit-tests.el (log-edit-done-strip-cvs-lines-cvs,
log-edit-done-strip-cvs-lines-non-cvs):
Add test cases 'log-edit-done-strip-cvs-lines'.
---
 lisp/vc/log-edit.el            | 16 +++++++++++++--
 test/lisp/vc/log-edit-tests.el | 37 ++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/log-edit.el b/lisp/vc/log-edit.el
index e23e7414a18..cb7c3565ccf 100644
--- a/lisp/vc/log-edit.el
+++ b/lisp/vc/log-edit.el
@@ -202,15 +202,17 @@ log-edit-mode-hook
   :group 'log-edit
   :type 'hook)
 
-(defcustom log-edit-done-hook nil
+(defcustom log-edit-done-hook '(log-edit-done-strip-cvs-lines)
   "Hook run before doing the actual commit.
 This hook can be used to cleanup the message, enforce various
 conventions, or to allow recording the message in some other database,
 such as a bug-tracking system.  The list of files about to be committed
 can be obtained from `log-edit-files'."
+  :version "30.1"
   :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 +928,16 @@ 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 interactively do this only when the VC backend is CVS."
+  (interactive)
+  (when (or (called-interactively-p)
+            (eq log-edit-vc-backend 'CVS))
+    (let ((search-upper-case nil))
+      (goto-char (point-min))
+      (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..e0e8d3e677a 100644
--- a/test/lisp/vc/log-edit-tests.el
+++ b/test/lisp/vc/log-edit-tests.el
@@ -360,4 +360,41 @@ log-edit-fill-entry-no-defun-list-wrapping
       (let ((fill-column 64)) (log-edit-fill-entry))
       (should (equal (buffer-string) wanted)))))
 
+(ert-deftest log-edit-done-strip-cvs-lines-cvs ()
+  ;; 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
+")
+    (with-temp-buffer
+      (let ((log-edit-done-hook 'log-edit-done-strip-cvs-lines)
+            (log-edit-vc-backend 'CVS))
+        (setq-local log-edit-callback #'(lambda () (interactive) nil))
+        (insert string)
+        (log-edit-done)
+        (should (equal (buffer-string) wanted))))))
+
+(ert-deftest log-edit-done-strip-cvs-lines-non-cvs ()
+  ;; 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.
+")
+    (with-temp-buffer
+      (let ((log-edit-done-hook 'log-edit-done-strip-cvs-lines)
+            (log-edit-vc-backend nil))
+        (setq-local log-edit-callback #'(lambda () (interactive) nil))
+        (insert string)
+        (log-edit-done)
+        (should (equal (buffer-string) string))))))
+
 ;;; log-edit-tests.el ends here
-- 
2.47.1