GNU bug report logs - #14303
24.3; Bug in comment-search-backward

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 14303 in the body.
You can then email your comments to 14303 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: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3; Bug in comment-search-backward
Date: Mon, 29 Apr 2013 21:27:35 +0800

Open a new buffer in octave-mode and insert the following line:

  x="#abc"

Move point to the end of the inserted line and

  M-: (comment-search-backward)

this moves point inside the string.

Due to this bug octave-mode users are seeing mysterious comment char
such as % or # inserted by fill-paragraph or auto-fill-mode.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: 14303 <at> debbugs.gnu.org
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Wed, 15 May 2013 19:33:23 +0800

On 2013-04-29 21:27 +0800, Leo Liu wrote:
> Open a new buffer in octave-mode and insert the following line:
>
>   x="#abc"
>
> Move point to the end of the inserted line and
>
>   M-: (comment-search-backward)
>
> this moves point inside the string.

So it is no longer fine for comment-search-backward to end up in a
comment or string any more.

How about something like this?

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index d55feaa3..65182c1b 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -485,27 +485,25 @@ (defun comment-search-backward (&optional limit noerror)
 Moves point to inside the comment and returns the position of the
 comment-starter.  If no comment is found, moves point to LIMIT
 and raises an error or returns nil if NOERROR is non-nil."
-  ;; FIXME: If a comment-start appears inside a comment, we may erroneously
-  ;; stop there.  This can be rather bad in general, but since
-  ;; 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))
-      (unless noerror (error "No comment"))
-    (beginning-of-line)
-    (let* ((end (match-end 0))
-	   (cs (comment-search-forward end t))
-	   (pt (point)))
-      (if (not cs)
-	  (progn (beginning-of-line)
-		 (comment-search-backward limit noerror))
-	(while (progn (goto-char cs)
-		      (comment-forward)
-		      (and (< (point) end)
-			   (setq cs (comment-search-forward end t))))
-	  (setq pt (point)))
-	(goto-char pt)
-	cs))))
+  (let (found end)
+    (while (and (not found) (re-search-backward comment-start-skip limit t))
+      (setq end (match-end 0))
+      (or (nth 8 (syntax-ppss)) (setq found t)))
+    (if (not found)
+	(unless noerror (error "No comment"))
+      (beginning-of-line)
+      (let ((cs (comment-search-forward end t))
+	    (pt (point)))
+	(if (not cs)
+	    (progn (beginning-of-line)
+		   (comment-search-backward limit noerror))
+	  (while (progn (goto-char cs)
+			(comment-forward)
+			(and (< (point) end)
+			     (setq cs (comment-search-forward end t))))
+	    (setq pt (point)))
+	  (goto-char pt)
+	  cs)))))
 
 (defun comment-beginning ()
   "Find the beginning of the enclosing comment.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Wed, 15 May 2013 18:13:47 +0200

Am 15.05.2013 13:33, schrieb Leo Liu:
> On 2013-04-29 21:27 +0800, Leo Liu wrote:
>> Open a new buffer in octave-mode and insert the following line:
>>
>>    x="#abc"
>>
>> Move point to the end of the inserted line and
>>
>>    M-: (comment-search-backward)
>>
>> this moves point inside the string.
>
> So it is no longer fine for comment-search-backward to end up in a
> comment or string any more.
>
> How about something like this?
>
> diff --git a/lisp/newcomment.el b/lisp/newcomment.el
> index d55feaa3..65182c1b 100644
> --- a/lisp/newcomment.el
> +++ b/lisp/newcomment.el
> @@ -485,27 +485,25 @@ (defun comment-search-backward (&optional limit noerror)
>   Moves point to inside the comment and returns the position of the
>   comment-starter.  If no comment is found, moves point to LIMIT
>   and raises an error or returns nil if NOERROR is non-nil."
> -  ;; FIXME: If a comment-start appears inside a comment, we may erroneously
> -  ;; stop there.  This can be rather bad in general, but since
> -  ;; 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))
> -      (unless noerror (error "No comment"))
> -    (beginning-of-line)
> -    (let* ((end (match-end 0))
> -	   (cs (comment-search-forward end t))
> -	   (pt (point)))
> -      (if (not cs)
> -	  (progn (beginning-of-line)
> -		 (comment-search-backward limit noerror))
> -	(while (progn (goto-char cs)
> -		      (comment-forward)
> -		      (and (< (point) end)
> -			   (setq cs (comment-search-forward end t))))
> -	  (setq pt (point)))
> -	(goto-char pt)
> -	cs))))
> +  (let (found end)
> +    (while (and (not found) (re-search-backward comment-start-skip limit t))
> +      (setq end (match-end 0))
> +      (or (nth 8 (syntax-ppss)) (setq found t)))
> +    (if (not found)
> +	(unless noerror (error "No comment"))
> +      (beginning-of-line)
> +      (let ((cs (comment-search-forward end t))
> +	    (pt (point)))
> +	(if (not cs)
> +	    (progn (beginning-of-line)
> +		   (comment-search-backward limit noerror))
> +	  (while (progn (goto-char cs)
> +			(comment-forward)
> +			(and (< (point) end)
> +			     (setq cs (comment-search-forward end t))))
> +	    (setq pt (point)))
> +	  (goto-char pt)
> +	  cs)))))
>
>   (defun comment-beginning ()
>     "Find the beginning of the enclosing comment.
>
>
>
>

syntax-ppss is reliable, while re-search-backward comment-start-skip might stop inside a string etc.

backward-line, end-of-line
if nt4 and nth8, goto char nth8

that's nearly all
as done consider limit of search, sure.

Watching with interest,

Andreas

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Thu, 16 May 2013 12:02:16 +0800

On 2013-05-16 00:13 +0800, Andreas Röhler wrote:
> syntax-ppss is reliable, while re-search-backward comment-start-skip
> might stop inside a string etc.
>
> backward-line, end-of-line
> if nt4 and nth8, goto char nth8
>
> that's nearly all
> as done consider limit of search, sure.
>
> Watching with interest,
>
> Andreas

I don't know what to make of this comment. Do you see a problem in the
patch?

Thanks,
Leo

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Thu, 16 May 2013 09:12:29 +0200

Am 16.05.2013 06:02, schrieb Leo Liu:
> On 2013-05-16 00:13 +0800, Andreas Röhler wrote:
>> syntax-ppss is reliable, while re-search-backward comment-start-skip
>> might stop inside a string etc.
>>
>> backward-line, end-of-line
>> if nt4 and nth8, goto char nth8
>>
>> that's nearly all
>> as done consider limit of search, sure.
>>
>> Watching with interest,
>>
>> Andreas
>
> I don't know what to make of this comment. Do you see a problem in the
> patch?
>

Yes, same thing as with beg-of-defun discussed elsewhere.

+ (while (and (not found) (re-search-backward comment-start-skip limit t))
+      (setq end (match-end 0))
+      (or (nth 8 (syntax-ppss)) (setq found t)))

This might find a comment-start inside a string.

Rely at (syntax-ppss)

if nt4 and nth8, goto char nth8


Cheers,

Andreas
> Thanks,
> Leo
>

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Thu, 16 May 2013 09:28:49 -0400

> So it is no longer fine for comment-search-backward to end up in a
> comment or string any more.

I'd like to know a bit more about "no longer": what has changed?

IIUC the change is that you want to use comment-search-backward in
a different circumstance, but I don't really understand what is
that circumstance.

> How about something like this?

It looks OK, though please only use syntax-ppss if comment-use-syntax is
set, because some modes still use comment commands in contexts where the
syntax tables do not reflect the intended comment syntax.


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Thu, 16 May 2013 23:50:33 +0800

On 2013-05-16 21:28 +0800, Stefan Monnier wrote:
> I'd like to know a bit more about "no longer": what has changed?
>
> IIUC the change is that you want to use comment-search-backward in
> a different circumstance, but I don't really understand what is
> that circumstance.

In octave mode, insert the following line:

printf ("aaaa dddd dddd dddd dddd dddd dddd dddd dddd dddd dddd #i", abcd)

move point to the beginning of last word 'abcd' and type M-j.

Also people have been observing weird behaviour in auto-fill-mode or
fill-paragraph of comment chars insertion that seems come from nowhere.
For example, in early org mode versions (probably version 4 or so) there
have been constant reports of such behaviours.

Leo

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Thu, 16 May 2013 13:38:29 -0400

> In octave mode, insert the following line:
> printf ("aaaa dddd dddd dddd dddd dddd dddd dddd dddd dddd dddd #i", abcd)
> move point to the beginning of last word 'abcd' and type M-j.

Right, that makes sense.


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 08:35:35 +0800

On 2013-05-17 01:38 +0800, Stefan Monnier wrote:
> Right, that makes sense.
>
>
>         Stefan

In that case I plan to install the patch as attached.

I have found that it is very easy for people who provide customised
comment-start-skip to introduce bugs. For example octave mode used to
have "\\s<+\\s-*" as comment start skip and was lucky to work in the
buggy comment-search-backward.

I wonder what to do here. Better documentation on comment-start-skip to
inform people that it is used both in re-search-forward/backward and
better be anchored properly?


diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index d55feaa3..db07e6a9 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -485,27 +485,26 @@ (defun comment-search-backward (&optional limit noerror)
 Moves point to inside the comment and returns the position of the
 comment-starter.  If no comment is found, moves point to LIMIT
 and raises an error or returns nil if NOERROR is non-nil."
-  ;; FIXME: If a comment-start appears inside a comment, we may erroneously
-  ;; stop there.  This can be rather bad in general, but since
-  ;; 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))
-      (unless noerror (error "No comment"))
-    (beginning-of-line)
-    (let* ((end (match-end 0))
-	   (cs (comment-search-forward end t))
-	   (pt (point)))
-      (if (not cs)
-	  (progn (beginning-of-line)
-		 (comment-search-backward limit noerror))
-	(while (progn (goto-char cs)
-		      (comment-forward)
-		      (and (< (point) end)
-			   (setq cs (comment-search-forward end t))))
-	  (setq pt (point)))
-	(goto-char pt)
-	cs))))
+  (let (found end)
+    (while (and (not found) (re-search-backward comment-start-skip limit t))
+      (setq end (match-end 0))
+      (unless (and comment-use-syntax (nth 8 (syntax-ppss)))
+	(setq found t)))
+    (if (not found)
+	(unless noerror (error "No comment"))
+      (beginning-of-line)
+      (let ((cs (comment-search-forward end t))
+	    (pt (point)))
+	(if (not cs)
+	    (progn (beginning-of-line)
+		   (comment-search-backward limit noerror))
+	  (while (progn (goto-char cs)
+			(comment-forward)
+			(and (< (point) end)
+			     (setq cs (comment-search-forward end t))))
+	    (setq pt (point)))
+	  (goto-char pt)
+	  cs)))))
 
 (defun comment-beginning ()
   "Find the beginning of the enclosing comment.
-- 
1.8.2

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 18:47:48 +0800

On 2013-05-16 15:12 +0800, Andreas Röhler wrote:
> Yes, same thing as with beg-of-defun discussed elsewhere.
>
> + (while (and (not found) (re-search-backward comment-start-skip limit t))
> +      (setq end (match-end 0))
> +      (or (nth 8 (syntax-ppss)) (setq found t)))
>
> This might find a comment-start inside a string.
>
> Rely at (syntax-ppss)
>
> if nt4 and nth8, goto char nth8

There is possibility of ending up in a string. How about something along
these lines? Thanks. Leo

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index d55feaa3..79cdc393 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -485,27 +485,30 @@ (defun comment-search-backward (&optional limit noerror)
 Moves point to inside the comment and returns the position of the
 comment-starter.  If no comment is found, moves point to LIMIT
 and raises an error or returns nil if NOERROR is non-nil."
-  ;; FIXME: If a comment-start appears inside a comment, we may erroneously
-  ;; stop there.  This can be rather bad in general, but since
-  ;; 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))
-      (unless noerror (error "No comment"))
-    (beginning-of-line)
-    (let* ((end (match-end 0))
-	   (cs (comment-search-forward end t))
-	   (pt (point)))
-      (if (not cs)
-	  (progn (beginning-of-line)
-		 (comment-search-backward limit noerror))
-	(while (progn (goto-char cs)
-		      (comment-forward)
-		      (and (< (point) end)
-			   (setq cs (comment-search-forward end t))))
-	  (setq pt (point)))
-	(goto-char pt)
-	cs))))
+  (let (found end beg)
+    (while (and (not found) (re-search-backward comment-start-skip limit t))
+      (setq beg (or (match-end 1) (match-beginning 0))
+	    end (match-end 0))
+      (when (or (not comment-use-syntax)
+		(and (not (nth 8 (syntax-ppss beg)))
+		     (nth 4 (syntax-ppss end))))
+	(setq found t))
+      (goto-char beg))
+    (if (not found)
+	(unless noerror (error "No comment"))
+      (beginning-of-line)
+      (let ((cs (comment-search-forward end t))
+	    (pt (point)))
+	(if (not cs)
+	    (progn (beginning-of-line)
+		   (comment-search-backward limit noerror))
+	  (while (progn (goto-char cs)
+			(comment-forward)
+			(and (< (point) end)
+			     (setq cs (comment-search-forward end t))))
+	    (setq pt (point)))
+	  (goto-char pt)
+	  cs)))))
 
 (defun comment-beginning ()
   "Find the beginning of the enclosing comment.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 13:14:58 +0200

Am 17.05.2013 12:47, schrieb Leo Liu:
> On 2013-05-16 15:12 +0800, Andreas Röhler wrote:
>> Yes, same thing as with beg-of-defun discussed elsewhere.
>>
>> + (while (and (not found) (re-search-backward comment-start-skip limit t))
>> +      (setq end (match-end 0))
>> +      (or (nth 8 (syntax-ppss)) (setq found t)))
>>
>> This might find a comment-start inside a string.
>>
>> Rely at (syntax-ppss)
>>
>> if nt4 and nth8, goto char nth8
>
> There is possibility of ending up in a string. How about something along
> these lines? Thanks. Leo
>
> diff --git a/lisp/newcomment.el b/lisp/newcomment.el
> index d55feaa3..79cdc393 100644
> --- a/lisp/newcomment.el
> +++ b/lisp/newcomment.el
> @@ -485,27 +485,30 @@ (defun comment-search-backward (&optional limit noerror)
>   Moves point to inside the comment and returns the position of the
>   comment-starter.  If no comment is found, moves point to LIMIT
>   and raises an error or returns nil if NOERROR is non-nil."
> -  ;; FIXME: If a comment-start appears inside a comment, we may erroneously
> -  ;; stop there.  This can be rather bad in general, but since
> -  ;; 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))
> -      (unless noerror (error "No comment"))
> -    (beginning-of-line)
> -    (let* ((end (match-end 0))
> -	   (cs (comment-search-forward end t))
> -	   (pt (point)))
> -      (if (not cs)
> -	  (progn (beginning-of-line)
> -		 (comment-search-backward limit noerror))
> -	(while (progn (goto-char cs)
> -		      (comment-forward)
> -		      (and (< (point) end)
> -			   (setq cs (comment-search-forward end t))))
> -	  (setq pt (point)))
> -	(goto-char pt)
> -	cs))))
> +  (let (found end beg)
> +    (while (and (not found) (re-search-backward comment-start-skip limit t))
> +      (setq beg (or (match-end 1) (match-beginning 0))
> +	    end (match-end 0))
> +      (when (or (not comment-use-syntax)
> +		(and (not (nth 8 (syntax-ppss beg)))
> +		     (nth 4 (syntax-ppss end))))
> +	(setq found t))
> +      (goto-char beg))
> +    (if (not found)
> +	(unless noerror (error "No comment"))
> +      (beginning-of-line)
> +      (let ((cs (comment-search-forward end t))
> +	    (pt (point)))
> +	(if (not cs)
> +	    (progn (beginning-of-line)
> +		   (comment-search-backward limit noerror))
> +	  (while (progn (goto-char cs)
> +			(comment-forward)
> +			(and (< (point) end)
> +			     (setq cs (comment-search-forward end t))))
> +	    (setq pt (point)))
> +	  (goto-char pt)
> +	  cs)))))
>
>   (defun comment-beginning ()
>     "Find the beginning of the enclosing comment.
>


The succession of things doesn't look right yet.

if (eq comment-use-syntax nil)

re-search-backward based solution

which is very seldom. Grep shows 4 cases.

Otherwise syntax-ppss - , prog1 and nth 4 goto char nth 8 - or so.
Make sure wrong regexp isn't called then/no wrong matches.

Andreas

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 19:34:17 +0800

On 2013-05-17 19:14 +0800, Andreas Röhler wrote:
> The succession of things doesn't look right yet.
>
> if (eq comment-use-syntax nil)
>
> re-search-backward based solution
>
> which is very seldom. Grep shows 4 cases.
>
> Otherwise syntax-ppss - , prog1 and nth 4 goto char nth 8 - or so.
> Make sure wrong regexp isn't called then/no wrong matches.
>
> Andreas

There is possible optimisation when 'end' is in a string. Other than
that do you have a case where my solution will fail?

Leo

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 14:39:17 +0200

Am 17.05.2013 13:34, schrieb Leo Liu:
> On 2013-05-17 19:14 +0800, Andreas Röhler wrote:
>> The succession of things doesn't look right yet.
>>
>> if (eq comment-use-syntax nil)
>>
>> re-search-backward based solution
>>
>> which is very seldom. Grep shows 4 cases.
>>
>> Otherwise syntax-ppss - , prog1 and nth 4 goto char nth 8 - or so.
>> Make sure wrong regexp isn't called then/no wrong matches.
>>
>> Andreas
>
> There is possible optimisation when 'end' is in a string.

Don't understand. "end" can't be in a string, if syntax-ppss is used.


 Other than
> that do you have a case where my solution will fail?

Not just fail.
However most if this code looks redundant, useless employing of re-search-..., which will slow down Emacs when called from a program.



>
> Leo
>

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 14303 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 21:18:10 +0800

On 2013-05-17 20:39 +0800, Andreas Röhler wrote:
> Don't understand. "end" can't be in a string, if syntax-ppss is used.
>
>
>  Other than
>> that do you have a case where my solution will fail?
>
> Not just fail.
> However most if this code looks redundant, useless employing of
> re-search-..., which will slow down Emacs when called from a program.

Much as I would like to incorporate your suggestion I couldn't do so
without fully understand your messages. I think we have two options:

1. if you have copyright assignment to FSF, post your patch and I will
   happily withdraw mine.

2. If not, leave my patch for Stefan to review.

Leo

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 09:26:37 -0400

> +    (while (and (not found) (re-search-backward comment-start-skip limit t))
> +      (setq end (match-end 0))
> +      (unless (and comment-use-syntax (nth 8 (syntax-ppss)))
> +	(setq found t)))

BTW, a useful idiom for such loops is the "empty body" while loop.

      (while (and (re-search-backward comment-start-skip limit t)
                  (progn
                    (setq end (match-end 0))
                    (and comment-use-syntax (nth 8 (syntax-ppss))))))

-- Stefan

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org,
	Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 09:28:04 -0400

>> Yes, same thing as with beg-of-defun discussed elsewhere.
>> 
>> + (while (and (not found) (re-search-backward comment-start-skip limit t))
>> +      (setq end (match-end 0))
>> +      (or (nth 8 (syntax-ppss)) (setq found t)))
>> 
>> This might find a comment-start inside a string.
>> 
>> Rely at (syntax-ppss)
>> 
>> if nt4 and nth8, goto char nth8

> There is possibility of ending up in a string.

I don't understand when that can happen (when inside a string (nth
8 ppss) is also non-nil).


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 21:37:25 +0800

On 2013-05-17 21:28 +0800, Stefan Monnier wrote:
> I don't understand when that can happen (when inside a string (nth
> 8 ppss) is also non-nil).

I have

(defvar octave-comment-start-skip "\\(^\\|\\S<\\)\\(?:%!\\|\\s<+\\)\\s-*"
  "Octave-specific `comment-start-skip' (which see).")

and this could find "#abc" as comment start where BEG is outside of
strings and comments but END is in a string.

Maybe this is due to setting octave-comment-start-skip incorrectly.

I looked at comment-normalize-vars and see it uses:

  \\(\\(^\\|[^\\\n]\\)\\(\\\\\\\\\\)*\\)

as anchor but I don't understand fully.

Leo

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: bug-gnu-emacs <at> gnu.org
Cc: Leo <sdl.web <at> gmail.com>, Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 16:27:12 +0200

Am 17.05.2013 15:37, schrieb Leo Liu:
> On 2013-05-17 21:28 +0800, Stefan Monnier wrote:
>> I don't understand when that can happen (when inside a string (nth
>> 8 ppss) is also non-nil).
>
> I have
>
> (defvar octave-comment-start-skip "\\(^\\|\\S<\\)\\(?:%!\\|\\s<+\\)\\s-*"
>    "Octave-specific `comment-start-skip' (which see).")
>
> and this could find "#abc" as comment start where BEG is outside of
> strings and comments but END is in a string.
>
> Maybe this is due to setting octave-comment-start-skip incorrectly.
>
> I looked at comment-normalize-vars and see it uses:
>
>    \\(\\(^\\|[^\\\n]\\)\\(\\\\\\\\\\)*\\)
>
> as anchor but I don't understand fully.
>
> Leo
>
>
>
>

BTW what is the fastest way moving backward --searching comment-- when not inside a comment?

Thought at

(forward-line -1)
(end-of-line)
ppss-Check-for-Comment-again

maybe re-search-backward is as fast?

Andreas

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Fri, 17 May 2013 11:52:54 -0400

>> I don't understand when that can happen (when inside a string (nth
>> 8 ppss) is also non-nil).
> I have
> (defvar octave-comment-start-skip "\\(^\\|\\S<\\)\\(?:%!\\|\\s<+\\)\\s-*"
>   "Octave-specific `comment-start-skip' (which see).")
> and this could find "#abc" as comment start where BEG is outside of
> strings and comments but END is in a string.

Ah, I see.  That's easy to fix: just check the syntax-ppss state at the
position about which you care, i.e. (or (match-end 1) (match-beginning 0)),
rather than at the position at which re-search-backward puts you.

> Maybe this is due to setting octave-comment-start-skip incorrectly.

> I looked at comment-normalize-vars and see it uses:
>   \\(\\(^\\|[^\\\n]\\)\\(\\\\\\\\\\)*\\)
> as anchor but I don't understand fully.

No, this is to try and avoid mis-recognizing \# (and \\\#, but not \\#)
as a comment starter when \ is an escape character.


        Stefan

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 14303 <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Sat, 18 May 2013 06:51:31 +0800

On 2013-05-17 21:26 +0800, Stefan Monnier wrote:
> BTW, a useful idiom for such loops is the "empty body" while loop.
>
>       (while (and (re-search-backward comment-start-skip limit t)
>                   (progn
>                     (setq end (match-end 0))
>                     (and comment-use-syntax (nth 8 (syntax-ppss))))))

Yes, I use this occasionally too. Since we need to know if a comment
start is found I don't use it here.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Sat, 18 May 2013 06:52:52 +0800

On 2013-05-17 22:27 +0800, Andreas Röhler wrote:
> maybe re-search-backward is as fast?

Usually it is fast enough. I have experienced some slow cases in C-x [
but maybe that is due to really bad regexp.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 14303-done <at> debbugs.gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Sat, 18 May 2013 06:54:46 +0800

Fixed in trunk.

On 2013-05-17 23:52 +0800, Stefan Monnier wrote:
> Ah, I see.  That's easy to fix: just check the syntax-ppss state at the
> position about which you care, i.e. (or (match-end 1) (match-beginning 0)),
> rather than at the position at which re-search-backward puts you.

Thanks a lot.

Leo

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Sat, 18 May 2013 07:23:33 +0200

Am 18.05.2013 00:52, schrieb Leo Liu:
> On 2013-05-17 22:27 +0800, Andreas Röhler wrote:
>> maybe re-search-backward is as fast?
>
> Usually it is fast enough. I have experienced some slow cases in C-x [
> but maybe that is due to really bad regexp.
>
> Leo
>

As this is a very basic routine, IMO every thinking to make it as fast as possible is well invested.
BTW it's your merit having addressed that item.

So just for the record maybe:

Why start searching backward with a re-? Why not search comment-start string, which should travel much faster?

Andreas

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 14303 <at> debbugs.gnu.org, Leo Liu <sdl.web <at> gmail.com>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Mon, 20 May 2013 21:55:33 -0400

> Why start searching backward with a re-? Why not search comment-start
> string, which should travel much faster?

Fast is good, but that would be simply wrong.


        Stefan

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14303 <at> debbugs.gnu.org, Leo Liu <sdl.web <at> gmail.com>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Wed, 22 May 2013 12:58:07 +0200

Am 21.05.2013 03:55, schrieb Stefan Monnier:
>> Why start searching backward with a re-? Why not search comment-start
>> string, which should travel much faster?
>
> Fast is good, but that would be simply wrong.
>
>
>          Stefan
>

May you point me at a use-case, where comment-start-skip as regexp is needed?
I.e. a case where comment-start as string wouldn't do it.

Thanks,

Andreas

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 14303 <at> debbugs.gnu.org, Leo Liu <sdl.web <at> gmail.com>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Wed, 22 May 2013 11:58:55 -0400

> May you point me at a use-case, where comment-start-skip as regexp is needed?
> I.e. a case where comment-start as string wouldn't do it.

E.g. in C++, comment-start is typically "//" which won't find the
beginning of a /*...*/ comment.


        Stefan

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14303 <at> debbugs.gnu.org, Leo Liu <sdl.web <at> gmail.com>
Subject: Re: bug#14303: 24.3; Bug in comment-search-backward
Date: Wed, 22 May 2013 18:53:11 +0200

Am 22.05.2013 17:58, schrieb Stefan Monnier:
>> May you point me at a use-case, where comment-start-skip as regexp is needed?
>> I.e. a case where comment-start as string wouldn't do it.
>
> E.g. in C++, comment-start is typically "//" which won't find the
> beginning of a /*...*/ comment.
>
>
>          Stefan
>

Okay, see. Thanks.

With different ways to start a comment, the regexp seems inevitable.
However, emacs lisp and related should not need it.

Given it's not defined if not needed, the backward-search could speed up in this cases.


Andreas

Previous Next

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