GNU bug report logs - #72341
VC: CVS template lines not stripped when committing

Previous Next

Full log

Message #38 received at 72341 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Christoph Badura <bad <at> bsd.de>
Cc: 72341 <at> debbugs.gnu.org
Subject: Re: bug#72341: VC: CVS template lines not stripped when committing
Date: Mon, 27 Jan 2025 10:54:36 +0000

Hello,

On Mon 27 Jan 2025 at 12:08am +01, Christoph Badura wrote:

> 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.

Cool.

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

Unfortunately, I believe it's too late for that.

> 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.

This is more of a bugfix, so you can add one if you would like, but I'm
not sure it's necessary.

> +  :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."

I think it would be better to use a wrapper function that looks at
log-edit-vc-backend and have the command do it unconditionally.
called-interactively-p should be avoided if it can be avoided.

> +    (let ((search-upper-case nil))

I think it's more usual to bind case-fold-search not search-upper-case.

> 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))

These tests are fairly contrived because you bind log-edit-vc-backend
yourself instead of using an actual CVS repository.  (Not a blocker.)

-- 
Sean Whitton

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.