GNU bug report logs - #78925
31.0.50; ffap's filename prompt problem in remote files

Previous Next

Full log

View this message in rfc822 format

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 78925 <at> debbugs.gnu.org
Subject: bug#78925: 31.0.50; ffap's filename prompt problem in remote files, [PATCH] Add option 'ffap-prefer-remote-file' (bug#78925)
Date: Tue, 8 Jul 2025 18:19:39 +0800

[Message part 1 (text/plain, inline)]

On Tue, Jul 8, 2025 at 1:17 AM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> Liu Hui <liuhui1610 <at> gmail.com> writes:
>
> Hi,
>
> > The attached patch adds an option 'ffap-prefer-remote-file', which
> > defaults to nil. A non-nil value means ffap always finds remote files
> > for absolute filenames in remote case, instead of finding local files
> > first.
>
> Thank you for the patch. I don't use ffap myself, so just some comments
> from cursory review.

Thanks for your comments!

> > +---
> > +*** New user option 'ffap-prefer-remote-file'.
> > +If non-nil, ffap always finds remote files in buffers with remote
> > +'default-directory'.  If nil, ffap finds local files first for absolute
> > +filenames in above buffers.  The default is nil.
>
> Please spell it out FFAP.

Done.

> > -(defun ffap-file-exists-string (file &optional nomodify)
> > +(defun ffap-file-exists-string (file &optional nomodify remote-host)
>
> A remote file name is not only the host. There are also, at least, a
> method and a user. Please call this argument remote-prefix or alike.

Renamed to remote-prefix.

> > +     ((or (not file)                 ; quietly reject nil
> > +       (zerop (length file)))        ; and also ""
> > +      nil)
>
> aka ((zerop (length file)) nil)

Done.

> > +     ((and remote-host                  ; prepend remote host to file
> > +           (file-name-absolute-p file)
> > +           (setq file (concat remote-host file))
> > +           nil))
>
> What, if file is already remote? Shouldn't the check be
>
> --8<---------------cut here---------------start------------->8---
>      ((and remote-host                  ; prepend remote host to file
>            (file-name-absolute-p file)
>            (not (file-name-remote-p file))
>            (setq file (concat remote-host file))
>            nil))
> --8<---------------cut here---------------end--------------->8---

I examined the callers (e.g. ffap-file-at-point) and they have ruled
out the possibility that file is remote by ffap-file-remote-p. So it
wasn't checked again here. The docstring has been revised to clarify
the usage of the new argument.

> >  (require 'cl-lib)
> >  (require 'ert)
> > +(require 'tramp)
> >  (require 'ert-x)
> >  (require 'ffap)
>
> Why Tramp? I guess it isn't needed.

The comment of ert-remote-temporary-file-directory:

;; If this defvar is used in a test file, `tramp' shall be loaded
;; prior `ert-x'.  There is no default value on w32 systems, which
;; could work out of the box.

Otherwise, the remote test is skipped with 'make test/ffap-tests'.

> > +  (let* ((ffap-prefer-remote-file t)
> > +         (default-directory
> > +          (expand-file-name ert-remote-temporary-file-directory))
> > +         (test-file (expand-file-name "ffap-test" default-directory)))
>
> I would use (make-temp-file "ffap-test")

Done.

[0001-Add-option-ffap-prefer-remote-file-bug-78925.patch (text/x-patch, attachment)]

Previous Next

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