bug#14303: 24.3; Bug in comment-search-backward

bug#14303: 24.3; Bug in comment-search-backward

[Leo Liu]

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

bug#14303: 24.3; Bug in comment-search-backward

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

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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

bug#14303: 24.3; Bug in comment-search-backward

[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?

Thanks,
Leo

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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
>

bug#14303: 24.3; Bug in comment-search-backward

[Stefan Monnier]

> 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

bug#14303: 24.3; Bug in comment-search-backward

[Leo Liu]

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

bug#14303: 24.3; Bug in comment-search-backward

[Stefan Monnier]

> 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

bug#14303: 24.3; Bug in comment-search-backward

[Leo Liu]

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

bug#14303: 24.3; Bug in comment-search-backward

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

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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

bug#14303: 24.3; Bug in comment-search-backward

[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. Other than
that do you have a case where my solution will fail?

Leo

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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
>

bug#14303: 24.3; Bug in comment-search-backward

[Leo Liu]

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

bug#14303: 24.3; Bug in comment-search-backward

[Stefan Monnier]

> +    (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

bug#14303: 24.3; Bug in comment-search-backward

[Stefan Monnier]

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

bug#14303: 24.3; Bug in comment-search-backward

[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

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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

bug#14303: 24.3; Bug in comment-search-backward

[Stefan Monnier]

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

bug#14303: 24.3; Bug in comment-search-backward

[Leo Liu]

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

bug#14303: 24.3; Bug in comment-search-backward

[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

bug#14303: 24.3; Bug in comment-search-backward

[Leo Liu]

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

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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

bug#14303: 24.3; Bug in comment-search-backward

[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

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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

bug#14303: 24.3; Bug in comment-search-backward

[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

bug#14303: 24.3; Bug in comment-search-backward

[Andreas Röhler]

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