bug#24057: 25.1.50; ffap interprets comments beginning with "//" as file path

[Kaushal Modi]

On Mon, Jul 25, 2016 at 1:24 PM Eli Zaretskii <eliz@gnu.org> wrote:

Not sure which behavior you call "buggy".  I'm okay with
unconditionally skipping the comment leader string, such as "//" in
C/C++ case.  If you want to skip more than that, I think skipping the
rest should be optional, defaulting to off.  How's that "buggy"?

Thanks for the clarification. I thought that you were suggesting to have a defcustom that would either use or not use comment-search-forward in ffap-string-at-point with the default set such that the old (current) behavior of not skipping the comment starter at all is retained. I refer to this "not skipping the comment starter at all" behavior as buggy.

 

> How about we add the defcustom and set the default value so that this bug is fixed. And we wait for people
> using the master branch to report any issues caused by this. Also we let emacs-devel know that this
> defcustom is going in.

I don't want us to introduce backward-incompatible behavior, except
when strictly necessary.  In this case, skipping the initial "//" is
necessary, but skipping more slashes is not.

Understood. In that case, the code will have to be made more complex...

Here's a quick attempt .. Can you please have a look at it?

 (defun modi/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.

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

                      (region-beginning)

                    (save-excursion

                      (skip-chars-backward (car args))

                      (skip-chars-forward (nth 1 args) pt)

                      (point))))

             (end (if region-selected

                      (region-end)

                    (save-excursion

                      (skip-chars-forward (car args))

                      (skip-chars-backward (nth 2 args) pt)

                      (point))))

             beg-temp)

        ;; Ensure that if "//" is a valid comment starter in the current major

        ;; mode, then that is removed from the returned string.

        (when (and (null region-selected)

                   ;; Check if the first character is '/' and not part of the

                   ;; comment

                   (save-excursion

                     (goto-char beg)

                     (and (looking-at "/")

                          (null (nth 4 (syntax-ppss)))))

                   ;; Check if the second character is '/' and also not part of

                   ;; the comment

                   (setq beg-temp (+ 1 beg))

                   (if (> end beg-temp)

                       (save-excursion

                         (goto-char beg-temp)

                         (and (looking-at "/")

                              (null (nth 4 (syntax-ppss)))))

                     nil))

          ;; Check if the third character *is* part of the comment

          (setq beg-temp (+ 2 beg))

          (if (> end beg-temp)

              (save-excursion

                (goto-char beg-temp)

                (when (nth 4 (syntax-ppss))

                  (setq beg beg-temp)))))

        (message "beg = %d beg-temp = %S end = %d "

                 beg beg-temp end)

        (prog1

            (setq ffap-string-at-point

                  (buffer-substring-no-properties

                   (setcar ffap-string-at-point-region beg)

                   (setcar (cdr ffap-string-at-point-region) end)))

          (message "dbg: %S" ffap-string-at-point)

          )))

    (advice-add 'ffap-string-at-point :override #'modi/ffap-string-at-point)

--

Kaushal Modi