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

Previous Next

Full log

View this message in rfc822 format

From: Christoph Badura <bad <at> bsd.de>
To: 72341 <at> debbugs.gnu.org
Subject: bug#72341: VC: CVS template lines not stripped when committing
Date: Mon, 27 Jan 2025 21:17:36 +0100

Thanks to for the review by both you and Eli.

On Mon, Jan 27, 2025 at 10:54:36AM +0000, Sean Whitton wrote:
> On Mon 27 Jan 2025 at 12:08am +01, Christoph Badura wrote:
> > +(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.

I don't understand what you mean.  Wrapper function around what?  And what
command should do what exactly unconditionally?

> called-interactively-p should be avoided if it can be avoided.

I was going by what almost all of the other log-edit-*-hook function in
log-edit.el do.  Also I thought it would be convenient to be able to M-X
this function.  I did cal called-interactively-p incorrectly, though.

> > +    (let ((search-upper-case nil))
> I think it's more usual to bind case-fold-search not search-upper-case.

Changed.  I did use search-upper-case because the docstring for
flush-lines mentions that and the regexp does contain upper case
characters.  So all the conditions are true and I did not look further.

Do you think the docstring for flush-lines should mention case-fold-search
too?

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

Binding log-edit-vc-backend myself is deliberate.  I'm testing behavior
that is internal to log-edit and independent from an actual respository
(CVS or other).  This is about what happens when log-edit-done cleans up
the buffer with the commit message before handing it is handed over to
the backend for the actual committing.  (Setting log-edit-vc-backend is
part of the contract of calling log-edit from vc-log-edit.)

I did notice just now that a) the docstring for log-edit-vc-backend is
wrong, and b) the call to vc-diff in log-edit-diff-fileset no longer
matches the signature of vc-diff.

--chris

Previous Next

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