GNU bug report logs - #24057
25.1.50; ffap interprets comments beginning with "//" as file path

Previous Next

Full log

View this message in rfc822 format

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24057 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: bug#24057: 25.1.50; ffap interprets comments beginning with "//" as file path
Date: Tue, 26 Jul 2016 15:11:45 +0000

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

On Tue, Jul 26, 2016 at 10:42 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > Here a git formatted patch .. it's an optimized, generic version of the
> > snippet in my previous email. It's now "/" agnostic.
>
> Thanks.  Most of the patch looks like whitespace changes?
>

I selected the whole defun and did auto-ident (C-M-\). The submitted patch
was a result of that. I thought it's a good idea to update the indentation
of the whole function. May be the lisp-indent-function changed since this
function was updated last time?

In any case, here is the patch ignoring whitespace changes. Note of caution
that the indentation will look messed up when you merge this patch:

=====

diff --git a/lisp/ffap.el b/lisp/ffap.el
index 7013e6e..d1bca04 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -1097,33 +1097,73 @@ ffap-string-at-point

 (defun ffap-string-at-point (&optional mode)
   "Return a string of characters from around point.
+
 MODE (defaults to value of `major-mode') is a symbol used to look up
 string syntax parameters in `ffap-string-at-point-mode-alist'.
+
 If MODE is not found, we use `file' instead of MODE.
+
 If the region is active,return a string from the region.
-Sets the variable `ffap-string-at-point' and the variable
-`ffap-string-at-point-region'."
+
+If the point is in a comment, ensure that the returned string does not
contain
+the comment start characters (especially for major modes that have '//' as
+comment start characters).
+
+Sets variables `ffap-string-at-point' and `ffap-string-at-point-region'. "
   (let* ((args
           (cdr
            (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
                (assq 'file ffap-string-at-point-mode-alist))))
+         (region-selected (use-region-p))
          (pt (point))
- (beg (if (use-region-p)
+         (beg (if region-selected
                   (region-beginning)
                 (save-excursion
                   (skip-chars-backward (car args))
                   (skip-chars-forward (nth 1 args) pt)
                   (point))))
- (end (if (use-region-p)
+         (end (if region-selected
                   (region-end)
                 (save-excursion
                   (skip-chars-forward (car args))
                   (skip-chars-backward (nth 2 args) pt)
-  (point)))))
+                  (point))))
+         (beg-new beg))
+    ;; (message "ffap-string-at-point dbg: beg = %d end = %d" beg end)
+    ;; If the initial characters of the to-be-returned string are the
+    ;; current major mode's comment starter characters, *and* are not part
+    ;; of a comment, remove those from the returned string (Bug#24057).
+    ;; Example comments in `c-mode' (which considers lines beginning with
+    ;; "//" as comments):
+    ;;  //tmp - This is a comment. It does not contain any path reference.
+    ;;  ///tmp - This is a comment. The "/tmp" portion in that is a path.
+    ;;  ////tmp - This is a comment. The "//tmp" portion in that is a path.
+    (when (and
+           ;; Proceed if no region is selected by the user.
+           (null region-selected)
+           ;; Check if END character is part of a comment.
+           (save-excursion
+             (goto-char end)
+             (nth 4 (syntax-ppss))))
+      (save-excursion
+        ;; Increment BEG till point at BEG is in a comment too.
+        ;; (nth 4 (syntax-ppss)) will be nil for comment start characters
+        ;; (for example, for the "//" characters in `c-mode' line
comments).
+        (setq beg (catch 'break
+                    (while (< beg-new end)
+                      (goto-char beg-new)
+                      (if (nth 4 (syntax-ppss)) ; in a comment
+                          (throw 'break beg-new)
+                        (setq beg-new (1+ beg-new))))
+                    end)))) ; Set BEG to END if no throw happens
+    ;; (message "ffap-string-at-point dbg: beg = %d beg-new = %d" beg
beg-new)
     (setq ffap-string-at-point
           (buffer-substring-no-properties
            (setcar ffap-string-at-point-region beg)
-   (setcar (cdr ffap-string-at-point-region) end)))))
+           (setcar (cdr ffap-string-at-point-region) end)))
+    ;; (message "ffap-string-at-point dbg: ffap-string-at-point = %S"
+    ;;          ffap-string-at-point)
+    ffap-string-at-point))

 (defun ffap-string-around ()
   ;; Sometimes useful to decide how to treat a string.
=====

> +        (setq beg (catch 'break
> > +                    (while (< beg-new end)
> > +                      (goto-char beg-new)
> > +                      (if (nth 4 (syntax-ppss)) ; in a comment
> > +                          (throw 'break beg-new)
> > +                        (setq beg-new (1+ beg-new))))
> > +                    end)))) ; Set BEG to END if no throw happens
>
> Is there any problem with a more conventional while loop that stops
> when syntax-ppss has its 5th element non-nil?  Why did you need to use
> throw and catch here?
>

Throw and catch just feels more intuitive to me. This construct also sets
beg to end if no throw happens.
It's just a difference of style; feel free to refactor it.
-- 

Kaushal Modi

[Message part 2 (text/html, inline)]

Previous Next

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