Subject: bug#78769: 31.0.50; improve ffap-file-name-with-spaces

From: Madhu <[email-masked]>
Date: Thu, 12 Jun 2025 06:39:53 +0530 (IST)

This is with regard to:

*  Eli Zaretskii <[email-masked]> <[email-masked]>
Wrote on Sun, 08 Jun 2025 08:30:50 +0300
>> Date: Sun, 08 Jun 2025 08:46:26 +0530
>> From:  Madhu
>> there is the "recently" (i.e. within living memory, or within 5 years)
>> introduced ffap-file-name-with-spaces which helps but it breaks ffap on
>> files with "~" patterns like "~/tmp/foo bar/var" patterns and generally
>> breaks "~/tmp/".
>
> It's actually worse than that: Windows-style d:/foo/bar file names
> (which AFAIU were the motivation for the change) are also broken when
> this option is non-nil (and the test suite just codifies the incorrect
> results!).
>
> Please submit a bug report about this semi-broken "feature".  It must
> be improved.

The code paths were introduced in
* commit f3afb23d26b948cfa095b221ca32090a2858e8f1
|Author:     Jari Aalto <[email-masked]>
|AuthorDate: Sat Aug 15 12:11:41 2020 +0200
|Commit:     Lars Ingebrigtsen <[email-masked]>
|CommitDate: Sat Aug 15 12:11:41 2020 +0200
|
|    Add support for ffap guessing at file names containing spaces
|
|    * lisp/ffap.el (ffap-file-name-with-spaces): New variable (bug#8439).
|    (ffap-search-backward-file-end, ffap-search-forward-file-end)
|    (ffap-dir-separator-near-point): New functions.
|    (ffap-string-at-point): Use the variable and the new functions to
|    guess at files containing strings.

After a bit of investigation, there are a couple of points.

First I think my use case of completing unix file names and spaces can
be supported without touching the existing code paths which use
ffap-file-name-with-spaces:

This simlply involves extending with the space character the car of
the alist entry for `file' in ffap-string-at-point-mode-alist, when it
is used in ffap-string-at-point

(setq args
  (assq 'file ffap-string-at-point-mode-alist))

;; => (file "--:\\\\${}+<>@-Z_[:alpha:]~*?#" "{<@" "@>;.,!:}")

;; note the first three characters of (car args) is just obscure
;; notation which expands to "-./0123456789:"
;; (apply 'string (loop for i from ?\- to ?\: collect i))

For testing, I propose introducing a new variable

(defvar ffap-file-name-with-spaces-only t)

and modifying the uses in ffap-string-at-point like this:

```
-		  (skip-chars-forward (car args))
+		  (skip-chars-forward (if ffap-file-name-with-spaces-only
+                                          (concat (car args)  " ")
+                                        (car args)))
```

This seems to be enough for ffap on my local files

Additional Proposed change 1a: "%" be added to (car args) - as this
commoon in url-hexified filenames

2a. I haven't understand the windows related use cases of that
ffap-file-name-with-spaces targeted, but fixing recognition of
~/dir/file patterns can be hard coded by fixing up the code which
finds the `beg` point like this:

```
+                        (if (eql (char-after (1- (point))) ?~)
+                            (goto-char (1- (point))))
```

2b. A possible improvement for  for finding `end' point which seems
to work on the few test cases I tried is like this:
```
-	          (when (and ffap-file-name-with-spaces
+	          (when (and ffap-file-name-with-spaces ;nil
+                             (setq dir-separator (ffap-dir-separator-near-point))
```

With these changes the entire code for ffap-string-at-point looks like
this.  (Please let me know if you think I should just jave attached a
diff, as most if it is unchanged. If you can try this definition with
some ad-hoc testing and have any comments on how to go about this, I
can follow up with specific test cases for these changes. ---Madhu

```
(defvar ffap-file-name-with-spaces-only t)

(defun ffap-string-at-point (&optional mode)
  (let* (dir-separator
         (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
	          (if (and ffap-file-name-with-spaces ;nil
			   (memq mode '(nil file)))
		      (when (setq dir-separator (ffap-dir-separator-near-point))
		        (while (re-search-backward
			        (regexp-quote dir-separator)
			        (line-beginning-position) t)
		          (goto-char (match-beginning 0)))
                        (cl-assert (looking-at "/"))
                        (if (eql (char-after (1- (point))) ?~)
                            (goto-char (1- (point))))
                        )
                    (progn
                      (skip-chars-backward (if ffap-file-name-with-spaces-only
                                               (concat (car args)  " ")
                                             (car args)))
                      (skip-chars-forward (nth 1 args) pt)))
		  (point))))
         (end (if region-selected
		  (region-end)
		(save-excursion
		  (skip-chars-forward (if ffap-file-name-with-spaces-only
                                          (concat (car args)  " ")
                                        (car args)))
		  (skip-chars-backward (nth 2 args) pt)
	          (when (and ffap-file-name-with-spaces ;nil
                             (setq dir-separator (ffap-dir-separator-near-point))
			     (memq mode '(nil file)))
		    (ffap-search-forward-file-end dir-separator)
		    (ffap-search-backward-file-end dir-separator))
		  (point))))
         (region-len (- (max beg end) (min 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
             (nth 4 (syntax-ppss end))))
      ;; Move BEG to beginning of comment (after the comment start
      ;; characters), or END, whichever comes first.
      (save-excursion
        (let ((state (syntax-ppss beg)))
          ;; (nth 4 (syntax-ppss)) will be nil for comment start chars.
          (unless (nth 4 state)
            (parse-partial-sexp beg end nil nil state :commentstop)
            (setq beg (point))))))

    (if (and (natnump ffap-max-region-length)
             (< region-len ffap-max-region-length)) ; Bug#25243.
        (setf ffap-string-at-point-region (list beg end)
              ffap-string-at-point
              (buffer-substring-no-properties beg end))
      (setf ffap-string-at-point-region (list 1 1)
            ffap-string-at-point ""))))
```