GNU bug report logs - #28428
comment-search-backward with no comments

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28428 in the body.
You can then email your comments to 28428 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: "N. Raghavendra" <raghu <at> hri.res.in>
To: bug-gnu-emacs <at> gnu.org
Subject: comment-search-backward with no comments
Date: Tue, 12 Sep 2017 04:07:35 +0530

[Message part 1 (text/plain, inline)]

I am using Emacs 26.0.50.  In the cases that I tried, when there is no
comment in the portion of the buffer between the search limit and point,
`comment-search-backward' does not move point to the search limit before
raising an error.  This behaviour contradicts its docstring.  My message
to emacs-devel about this is at
http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00193.html

Attached are a patch that seems to fix the problem, and a file
containing a test.  Running the test without the change in the patch
fails, and passes with the change:

$ git checkout master

$ emacs -Q -batch -l lisp/newcomment.el -l /tmp/test.el \
  -f ert-run-tests-batch-and-exit
----------------------------------------------------------------------
(ert-test-failed
     ((should
       (=
        (point)
        limit))
      :form
      (= 70 41)
      :value nil))
   FAILED  1/1  test-comment-search-backward-absent
----------------------------------------------------------------------

$ git checkout test-comment-search-forward-absent

$ emacs -Q -batch -l lisp/newcomment.el -l /tmp/test.el \
  -f ert-run-tests-batch-and-exit
----------------------------------------------------------------------
Running 1 tests (2017-09-12 03:31:48+0530)
   passed  1/1  test-comment-search-backward-absent
----------------------------------------------------------------------

Raghu.

--
N. Raghavendra <raghu <at> hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

[0001-comment-search-backword-move-point-when-there-is-no-comment.patch (text/x-diff, inline)]

From eb8e32c6fe4a3d08be8b856b3ff4a300a2e69d4b Mon Sep 17 00:00:00 2001
From: "N. Raghavendra" <raghu <at> hri.res.in>
Date: Tue, 12 Sep 2017 01:41:16 +0530
Subject: [PATCH] comment-search-backword: move point when there is no comment

* lisp/newcomment.el (comment-search-backward): Move point to
LIMIT when there is no comment between LIMIT and point.
---
 lisp/newcomment.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index 8772b523..d849fc1 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -525,7 +525,9 @@ comment-search-backward
   ;; comment-set-column) and to find the comment-start string (via
   ;; comment-beginning) in indent-new-comment-line, it should be harmless.
   (if (not (re-search-backward comment-start-skip limit t))
-      (unless noerror (error "No comment"))
+      (progn
+        (goto-char limit)
+        (unless noerror (error "No comment")))
     (beginning-of-line)
     (let* ((end (match-end 0))
 	   (cs (comment-search-forward end t))
-- 
2.7.4

[test.el (application/emacs-lisp, attachment)]

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "N. Raghavendra" <raghu <at> hri.res.in>
Cc: 28428 <at> debbugs.gnu.org
Subject: Re: bug#28428: comment-search-backward with no comments
Date: Fri, 22 Sep 2017 12:40:58 -0400

>    (if (not (re-search-backward comment-start-skip limit t))
> -      (unless noerror (error "No comment"))
> +      (progn
> +        (goto-char limit)
> +        (unless noerror (error "No comment")))

That looks fine, thank you.  But I think the simpler patch below works
as well:

    diff --git a/lisp/newcomment.el b/lisp/newcomment.el
    index be6dbe3a4c..e32966e596 100644
    --- a/lisp/newcomment.el
    +++ b/lisp/newcomment.el
    @@ -524,7 +524,7 @@ comment-search-backward
       ;; comment-search-backward is only used to find the comment-column (in
       ;; comment-set-column) and to find the comment-start string (via
       ;; comment-beginning) in indent-new-comment-line, it should be harmless.
    -  (if (not (re-search-backward comment-start-skip limit t))
    +  (if (not (re-search-backward comment-start-skip limit 'move))
           (unless noerror (error "No comment"))
         (beginning-of-line)
         (let* ((end (match-end 0))

I wonder if there's code out there that depends on this behavior, tho,
since AFAIK it's been behaving this way "forever".


        Stefan

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

From: "N. Raghavendra" <raghu <at> hri.res.in>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 28428 <at> debbugs.gnu.org
Subject: Re: bug#28428: comment-search-backward with no comments
Date: Fri, 22 Sep 2017 23:18:22 +0530

At 2017-09-22T12:40:58-04:00, Stefan Monnier wrote:

> That looks fine, thank you.  But I think the simpler patch below works
> as well:
>
>     -  (if (not (re-search-backward comment-start-skip limit t))
>     +  (if (not (re-search-backward comment-start-skip limit 'move))

That's indeed nicer!  Thanks for pointing out that the NOERROR arg of
`re-search-backward' can be given a value outside {t,nil} to move point
to LIMIT.  It's a lesson to myself that I should read docstrings more
carefully.

> I wonder if there's code out there that depends on this behavior, tho,
> since AFAIK it's been behaving this way "forever".

As you say, it doesn't really matter; it's just that there is a
discrepancy between the docstring specification of the effects of the
function and the said effects, which would be good to remove.

I came across it because I've been playing around with a major mode
based on AUCTeX, and was writing ERT tests for some initialisation code
I was using from AUCTeX.  Perhaps such tests are the only places where
these trivial errors matter.

Regards,
Raghu.

-- 
N. Raghavendra <raghu <at> hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "N. Raghavendra" <raghu <at> hri.res.in>
Cc: 28428 <at> debbugs.gnu.org
Subject: Re: bug#28428: comment-search-backward with no comments
Date: Fri, 22 Sep 2017 15:26:58 -0400

>> That looks fine, thank you.  But I think the simpler patch below works
>> as well:
>> -  (if (not (re-search-backward comment-start-skip limit t))
>> +  (if (not (re-search-backward comment-start-skip limit 'move))
> That's indeed nicer!  Thanks for pointing out that the NOERROR arg of
> `re-search-backward' can be given a value outside {t,nil} to move point
> to LIMIT.  It's a lesson to myself that I should read docstrings more
> carefully.

I think it's too risky for the emacs-26 branch, but feel free to install
it on master,


        Stefan

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

From: "N. Raghavendra" <raghu <at> hri.res.in>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 28428 <at> debbugs.gnu.org
Subject: Re: bug#28428: comment-search-backward with no comments
Date: Sat, 23 Sep 2017 02:01:14 +0530

At 2017-09-22T15:26:58-04:00, Stefan Monnier wrote:

> I think it's too risky for the emacs-26 branch, but feel free to install
> it on master,

Do you mean that the change can be pushed to master?  If so, do you want
me to do anything about it?  I guess someone with commit access will
have to take over.

Raghu.

-- 
N. Raghavendra <raghu <at> hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Message #22 received at 28428-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "N. Raghavendra" <raghu <at> hri.res.in>
Cc: 28428-done <at> debbugs.gnu.org
Subject: Re: bug#28428: comment-search-backward with no comments
Date: Sat, 23 Sep 2017 17:07:15 -0400

Version: 27.1

I pushed the patch to `master`.


        Stefan

Message #25 received at 28428-done <at> debbugs.gnu.org (full text, mbox):

From: "N. Raghavendra" <raghu <at> hri.res.in>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 28428-done <at> debbugs.gnu.org
Subject: Re: bug#28428: comment-search-backward with no comments
Date: Sun, 24 Sep 2017 05:56:50 +0530

At 2017-09-23T17:07:15-04:00, Stefan Monnier wrote:

> I pushed the patch to `master`.

Perfect!

Thanks,
Raghu.

-- 
N. Raghavendra <raghu <at> hri.res.in>, http://www.retrotexts.net/
Harish-Chandra Research Institute, http://www.hri.res.in/

Previous Next

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