copyright-fix-years seems broken

[Lele Gaifax]

Hi all,

I tried using the `copyright-fix-years' function on a source that have a very
long sequence of "Copyright (C) x, y, z, ..." years, and got the wrong result:

  # Copyright (C) 2008, 2009, 2010, 2013, 2014, 2015, 2016, 2018, 2019, 2020, 
2021 Lele Gaifax

produced

  # Copyright (C) 2008 2010, 2013 2016, 2018-2021 Lele Gaifax

With edebug, I found that the culprit seems to be the following code:

  (when (and first-year prev-year
             (> prev-year first-year))
    (goto-char range-end)
    (delete-region range-start range-end)
    (insert (format "%c%d" sep prev-year))
    (goto-char p))

where `sep' is indeed a space, defined as `(setq sep (char-before)' a dozen of
lines above, when the point is at the beginning of a matched year.

I then wrote the following test case, that indeed exhibits the problem:

  diff --git a/test/lisp/emacs-lisp/copyright-tests.el 
b/test/lisp/emacs-lisp/copyright-tests.el
  index dc82974a99..c55e19b064 100644
  --- a/test/lisp/emacs-lisp/copyright-tests.el
  +++ b/test/lisp/emacs-lisp/copyright-tests.el
  @@ -50,5 +50,23 @@ copyright-tests--data
     (dolist (test copyright-tests--data)
       (with-copyright-test (car test) (cdr test))))

  +(defmacro with-copyright-fix-years-test (orig result)
  +  `(let ((copyright-year-ranges t))
  +     (with-temp-buffer
  +       (insert ,orig)
  +       (copyright-fix-years)
  +       (should (equal (buffer-string) ,result)))))
  +
  +(defvar copyright-fix-years-tests--data
  +  '((";; Copyright (C) 2008, 2010, 2012"
  +     . ";; Copyright (C) 2008, 2010, 2012")
  +    (";; Copyright (C) 2008, 2009, 2010, 2013, 2014, 2015, 2016, 2018"
  +     . ";; Copyright (C) 2008-2010, 2013-2016, 2018")))
  +
  +(ert-deftest text-copyright-fix-years ()
  +  "Test basics of \\[copyright-fix-years]."
  +  (dolist (test copyright-fix-years-tests--data)
  +    (with-copyright-fix-years-test (car test) (cdr test))))
  +
   (provide 'copyright-tests)
   ;;; copyright-tests.el ends here

This seems to cure the issue:

  diff --git a/lisp/emacs-lisp/copyright.el b/lisp/emacs-lisp/copyright.el
  index 09c6ded295..f5e502e291 100644
  --- a/lisp/emacs-lisp/copyright.el
  +++ b/lisp/emacs-lisp/copyright.el
  @@ -311,7 +311,7 @@ copyright-fix-years
                             (> prev-year first-year))
                    (goto-char range-end)
                    (delete-region range-start range-end)
  -               (insert (format "%c%d" sep prev-year))
  +               (insert (format "-%d" prev-year))
                    (goto-char p))
                  (setq first-year year
                        range-start (point)))))

Before sending a proper bug report with the complete patch, could you tell me
if that is the the right way to write such test (I'm not sure about the new
macro...)?

Thanks in advance,
bye, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.