GNU bug report logs - #78769
31.0.50; improve ffap-file-name-with-spaces

Previous Next

To reply to this bug, email your comments to 78769 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Madhu <enometh <at> meer.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; improve ffap-file-name-with-spaces
Date: Thu, 12 Jun 2025 06:39:53 +0530 (IST)

This is with regard to:

*  Eli Zaretskii <eliz <at> gnu.org> <86bjqyhkad.fsf <at> gnu.org>
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 <jari.aalto <at> cante.net>
|AuthorDate: Sat Aug 15 12:11:41 2020 +0200
|Commit:     Lars Ingebrigtsen <larsi <at> gnus.org>
|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 ""))))
```

Message #8 received at 78769 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Madhu <enometh <at> meer.net>
Cc: 78769 <at> debbugs.gnu.org
Subject: Re: bug#78769: 31.0.50; improve ffap-file-name-with-spaces
Date: Thu, 12 Jun 2025 09:19:31 +0300

> Date: Thu, 12 Jun 2025 06:39:53 +0530 (IST)
> From:  Madhu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> This is with regard to:
> 
> *  Eli Zaretskii <eliz <at> gnu.org> <86bjqyhkad.fsf <at> gnu.org>
> 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 <jari.aalto <at> cante.net>
> |AuthorDate: Sat Aug 15 12:11:41 2020 +0200
> |Commit:     Lars Ingebrigtsen <larsi <at> gnus.org>
> |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))

Thanks.  I meanwhile installed on the master branch of the Emacs Git
repository, 4 days ago, a change which should fix the problem you
reported.  If you are using the master branch, updating from Git
should solve these problems for you.  Could you please try the latest
master branch and see if it solves this problem?

Message #11 received at 78769 <at> debbugs.gnu.org (full text, mbox):

From: Madhu <enometh <at> meer.net>
To: eliz <at> gnu.org
Cc: 78769 <at> debbugs.gnu.org
Subject: Re: bug#78769: 31.0.50; improve ffap-file-name-with-spaces
Date: Fri, 13 Jun 2025 07:18:52 +0530 (IST)

*  Eli Zaretskii <eliz <at> gnu.org> <867c1hh47g.fsf <at> gnu.org>
Wrote on Thu, 12 Jun 2025 09:19:31 +0300
> Thanks.  I meanwhile installed on the master branch of the Emacs Git
> repository, 4 days ago, a change which should fix the problem you
> reported.  If you are using the master branch, updating from Git
> should solve these problems for you.  Could you please try the latest
> master branch and see if it solves this problem?

Yes, I applied the patch from commit ff8c0648fb4dd67bf and it fixes
the problem mentioned on the help mailing list with the variable
turned on.  Thanks. That was quick.

Message #16 received at 78769-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Madhu <enometh <at> meer.net>
Cc: 78769-done <at> debbugs.gnu.org
Subject: Re: bug#78769: 31.0.50; improve ffap-file-name-with-spaces
Date: Fri, 13 Jun 2025 09:44:48 +0300

> Date: Fri, 13 Jun 2025 07:18:52 +0530 (IST)
> Cc: 78769 <at> debbugs.gnu.org
> From: Madhu <enometh <at> meer.net>
> 
> *  Eli Zaretskii <eliz <at> gnu.org> <867c1hh47g.fsf <at> gnu.org>
> Wrote on Thu, 12 Jun 2025 09:19:31 +0300
> > Thanks.  I meanwhile installed on the master branch of the Emacs Git
> > repository, 4 days ago, a change which should fix the problem you
> > reported.  If you are using the master branch, updating from Git
> > should solve these problems for you.  Could you please try the latest
> > master branch and see if it solves this problem?
> 
> Yes, I applied the patch from commit ff8c0648fb4dd67bf and it fixes
> the problem mentioned on the help mailing list with the variable
> turned on.  Thanks. That was quick.

The problem bothered me a lot, so I worked on it right away.

Thanks for testing, I'm therefore closing this bug.

Message #19 received at 78769-done <at> debbugs.gnu.org (full text, mbox):

From: Madhu <enometh <at> meer.net>
To: eliz <at> gnu.org
Cc: 78769-done <at> debbugs.gnu.org
Subject: Re: bug#78769: 31.0.50; improve ffap-file-name-with-spaces
Date: Fri, 13 Jun 2025 12:35:52 +0530 (IST)

*  Eli Zaretskii <eliz <at> gnu.org> <86msacdtsv.fsf <at> gnu.org>
Wrote on Fri, 13 Jun 2025 09:44:48 +0300
> The problem bothered me a lot, so I worked on it right away.
>
> Thanks for testing, I'm therefore closing this bug.

Thanks, it may be worth mentioning that setting
ffap-file-name-with-spaces t has undsired effects on regular ffap. eg:
ffap on the /etc/passwd on the line

```
/etc/passwd /etc/hosts
```

will return /etc not the full path.

but this is a known limitation.

[Unfortunately it is also a limitation in the other implementation I
proposed, that of extending
 (assq 'file ffap-string-at-point-mode-alist)
so that doesn't buy anything.]

Message #22 received at 78769 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Madhu <enometh <at> meer.net>
Cc: 78769 <at> debbugs.gnu.org
Subject: Re: bug#78769: 31.0.50; improve ffap-file-name-with-spaces
Date: Fri, 13 Jun 2025 10:21:37 +0300

> Date: Fri, 13 Jun 2025 12:35:52 +0530 (IST)
> Cc: 78769-done <at> debbugs.gnu.org
> From: Madhu <enometh <at> meer.net>
> 
> *  Eli Zaretskii <eliz <at> gnu.org> <86msacdtsv.fsf <at> gnu.org>
> Wrote on Fri, 13 Jun 2025 09:44:48 +0300
> > The problem bothered me a lot, so I worked on it right away.
> >
> > Thanks for testing, I'm therefore closing this bug.
> 
> Thanks, it may be worth mentioning that setting
> ffap-file-name-with-spaces t has undsired effects on regular ffap. eg:
> ffap on the /etc/passwd on the line
> 
> ```
> /etc/passwd /etc/hosts
> ```
> 
> will return /etc not the full path.
> 
> but this is a known limitation.
> 
> [Unfortunately it is also a limitation in the other implementation I
> proposed, that of extending
>  (assq 'file ffap-string-at-point-mode-alist)
> so that doesn't buy anything.]

I think this aspect is already in the doc string of the variable:

  Enabling this variable may lead to ‘find-file-at-point’ guessing
  wrong more often when trying to find a file name intermingled
  with normal text, but can be useful when working on systems that
  normally use spaces in file names (like Microsoft Windows and the
  like).

Message #25 received at 78769 <at> debbugs.gnu.org (full text, mbox):

From: Madhu <enometh <at> meer.net>
To: eliz <at> gnu.org
Cc: 78769 <at> debbugs.gnu.org
Subject: Re: bug#78769: 31.0.50; improve ffap-file-name-with-spaces
Date: Sat, 14 Jun 2025 21:07:38 +0530 (IST)

*  Eli Zaretskii <eliz <at> gnu.org> <86ldpwds3i.fsf <at> gnu.org>
Wrote on Fri, 13 Jun 2025 10:21:37 +0300
>> [Unfortunately it is also a limitation in the other implementation I
>> proposed, that of extending
>>  (assq 'file ffap-string-at-point-mode-alist)
>> so that doesn't buy anything.]

> I think this aspect is already in the doc string of the variable:

Yes of course.  I actually forgot to mention a different limitation of
the existing implementation. If the file name doesn't have an
extension you lose. with the spaces flag set

eg. ffap on  ~/.emacs or /etc/profile will only show the directories.

(This limitation isn't there in the alternative I proposed),

just a note. no changes to bug status.

to make ffap use a prefix argument to set ffap-file-name-with-spaces
is pretty hairy because ffap rebinds current-prefix-arg to nil.

With newadvice it can look like this

(defvar ffap-spaces-override nil)

(defun find-file-at-point-munge-cpa-around-advice (orig-fn &optional file-name)
  (let ((ffap-spaces-override current-prefix-arg))
    (funcall orig-fn file-name)))

(defun ffap-string-at-point-force-spaces-around-advice (orig-fn &optional mode)
  (let ((ffap-file-name-with-spaces (if ffap-spaces-override t nil)))
    (funcall orig-fn mode)))

(advice-add 'find-file-at-point :around
	    'find-file-at-point-munge-cpa-around-advice)

(advice-add 'ffap-string-at-point :around
	    'ffap-string-at-point-force-spaces-around-advice)

(when nil
  (advice-remove 'find-file-at-point :around
	       'find-file-at-point-munge-cpa-around-advice))
(when nil
  (advice-remove 'ffap-string-at-point
		 'ffap-string-at-point-force-spaces-around-advice))

;; which gives one a headache

Previous Next

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