Package: emacs;
Reported by: Madhu <enometh <at> meer.net>
Date: Thu, 12 Jun 2025 01:11:02 UTC
Severity: normal
Found in version 31.0.50
Done: Eli Zaretskii <eliz <at> gnu.org>
View this message in rfc822 format
From: Madhu <enometh <at> meer.net> To: 78769 <at> debbugs.gnu.org Subject: bug#78769: 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 ""))))
```
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.