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

[Kaushal Modi <kaushal.modi@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4328 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 6530 bytes --]