bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Gregor Zattler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dear Emacs developers, following this recipe (in
some Gnu/Linux shell):

$ cd /tmp
tmp$ emacs -Q -nw
# type /ssh:user@server:/home/test
# type M-x ffap RET

the proposed file name in minibuffer then is:
/tmp/ssh:user@server:/home/test
instead of
/ssh:user@server:/home/test

Therefore it's not possible to just hit RET in order
to open that file.  But that would be nice.

Thanks for your attention, Gregor

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu,
 cairo version 1.16.0) of 2023-12-02 built on no
Repository revision: fb4b0b30a24e8192e71c1425dc8a295cf39f64ff
Repository branch: HEAD
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --infodir=/usr/share/info/emacs --with-json
 --with-file-notification=yes --with-libsystemd
 --with-cairo --with-x=yes --with-x-toolkit=no
 --without-toolkit-scroll-bars --without-gsettings
 --enable-checking=yes,glyphs
 --enable-check-lisp-object-type
 --with-native-compilation 'CFLAGS=-g3 -O3
 -ffile-prefix-map=/home/grfz/src/emacs-master--fb4b0b30a24e8192e71c1425dc8a295cf39f64ff--2023-12-02T22-47+01-00=. -fstack-protector-strong
 -Wformat -Werror=format-security ''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM
HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD
LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY
OLDXMENU PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS
TIFF WEBP X11 XDBE XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_ALL:
  value of $LC_COLLATE: de_DE.utf8
  value of $LC_CTYPE: de_DE.utf8
  value of $LC_MESSAGES: POSIX
  value of $LC_MONETARY: de_DE.utf8
  value of $LC_NUMERIC: de_DE.utf8
  value of $LC_TIME: de_DE.utf8
  value of $LANG: de_DE.utf8
  locale-coding-system: utf-8-unix

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Stephen Berman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Thu, 07 Dec 2023 14:45:34 +0100 Gregor Zattler via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:

> Dear Emacs developers, following this recipe (in
> some Gnu/Linux shell):
>
> $ cd /tmp
> tmp$ emacs -Q -nw
> # type /ssh:user@server:/home/test
> # type M-x ffap RET
>
> the proposed file name in minibuffer then is:
> /tmp/ssh:user@server:/home/test
> instead of
> /ssh:user@server:/home/test
>
> Therefore it's not possible to just hit RET in order
> to open that file.  But that would be nice.

I see the same thing if the cursor is anywhere in that line after the
first `/', but if the cursor is on the first `/', then for me `M-x ffap
RET' does enter "/ssh:user@server:/home/test" into the minibuffer.

Steve Berman

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Gregor Zattler via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Dear Emacs developers,

Hi Gregor,

> following this recipe (in
> some Gnu/Linux shell):
>
> $ cd /tmp
> tmp$ emacs -Q -nw
> # type /ssh:user@server:/home/test
> # type M-x ffap RET
>
> the proposed file name in minibuffer then is:
> /tmp/ssh:user@server:/home/test
> instead of
> /ssh:user@server:/home/test
>
> Therefore it's not possible to just hit RET in order
> to open that file.  But that would be nice.

I can reproduce the behavior for all Emacsen down to Emacs 25 (the
oldest I have a running copy of), so I guess it didn't work ever. The
problem is, that `ffap-guesser' returns "ssh:user@server:/home/test",
which looks like a relative file name. Therefore, it is expanded to the
default directory "/tmp/".

I have no idea about ffap.el and thingatpt.el, so I fear I cannot do too
much. Somebody else who knows these libraries might fix it.

> Thanks for your attention, Gregor

Best regards, Michael.

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Visuwesh]

[Friday December 08, 2023] Michael Albinus wrote:

> Gregor Zattler via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> Dear Emacs developers,
>
> Hi Gregor,
>
>> following this recipe (in
>> some Gnu/Linux shell):
>>
>> $ cd /tmp
>> tmp$ emacs -Q -nw
>> # type /ssh:user@server:/home/test
>> # type M-x ffap RET
>>
>> the proposed file name in minibuffer then is:
>> /tmp/ssh:user@server:/home/test
>> instead of
>> /ssh:user@server:/home/test
>>
>> Therefore it's not possible to just hit RET in order
>> to open that file.  But that would be nice.
>
> I can reproduce the behavior for all Emacsen down to Emacs 25 (the
> oldest I have a running copy of), so I guess it didn't work ever. 

You were quite close.  That was when ffap-lax-url was set to t, previous
versions would have done the right thing I think.

> The problem is, that `ffap-guesser' returns "ssh:user@server:/home/test",
> which looks like a relative file name. Therefore, it is expanded to the
> default directory "/tmp/".
>
> I have no idea about ffap.el and thingatpt.el, so I fear I cannot do too
> much. Somebody else who knows these libraries might fix it.

With the default value of ffap-lax-url=t,
thing-at-point-bounds-url-at-point does not check the scheme of the url
so it ends up matching ssh:user@server alone.  With the nil value, ffap
correctly suggests /ssh:user@server:/home/test because the correct
scheme is ssh:// not ssh:.
But with ffap-lax-url=nil, you lose the ability to insert
mailto:user@host when the point is on user@host because it is not a
"well-formed" url [1] and thusly does not get picked up by
thing-at-point-url-at-point.  I hope my explanation is understandable.

I'm not sure what to do here frankly.

Hmm... looking at the region history for the defcustom, it was changed
to t by Eli for bug#25264: the user wanted the email address like
user@host to be matched by ffap.  I suppose we can try to see if the
point is on an email address in ffap-guesser after URL and filename
guesses, then we can turn ffap-lax-url back to nil and everyone will be
happy.

1. See thing-at-point--bounds-of-well-formed-url.

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Visuwesh]

[Tuesday December 12, 2023] Visuwesh wrote:

> [Friday December 08, 2023] Michael Albinus wrote:
>
>> Gregor Zattler via "Bug reports for GNU Emacs, the Swiss army knife of
>> text editors" <bug-gnu-emacs@gnu.org> writes:
>>
>>> Dear Emacs developers,
>>
>> Hi Gregor,
>>
>>> following this recipe (in
>>> some Gnu/Linux shell):
>>>
>>> $ cd /tmp
>>> tmp$ emacs -Q -nw
>>> # type /ssh:user@server:/home/test
>>> # type M-x ffap RET
>>>
>>> the proposed file name in minibuffer then is:
>>> /tmp/ssh:user@server:/home/test
>>> instead of
>>> /ssh:user@server:/home/test
>>>
>>> Therefore it's not possible to just hit RET in order
>>> to open that file.  But that would be nice.
>>
>> I can reproduce the behavior for all Emacsen down to Emacs 25 (the
>> oldest I have a running copy of), so I guess it didn't work ever. 
>
> You were quite close.  That was when ffap-lax-url was set to t, previous
> versions would have done the right thing I think.
>
>> The problem is, that `ffap-guesser' returns "ssh:user@server:/home/test",
>> which looks like a relative file name. Therefore, it is expanded to the
>> default directory "/tmp/".
>>
>> I have no idea about ffap.el and thingatpt.el, so I fear I cannot do too
>> much. Somebody else who knows these libraries might fix it.
>
> With the default value of ffap-lax-url=t,
> thing-at-point-bounds-url-at-point does not check the scheme of the url
> so it ends up matching ssh:user@server alone.  With the nil value, ffap
> correctly suggests /ssh:user@server:/home/test because the correct
> scheme is ssh:// not ssh:.
> But with ffap-lax-url=nil, you lose the ability to insert
> mailto:user@host when the point is on user@host because it is not a
> "well-formed" url [1] and thusly does not get picked up by
> thing-at-point-url-at-point.  I hope my explanation is understandable.
>
> I'm not sure what to do here frankly.
>
> Hmm... looking at the region history for the defcustom, it was changed
> to t by Eli for bug#25264: the user wanted the email address like
> user@host to be matched by ffap.  I suppose we can try to see if the
> point is on an email address in ffap-guesser after URL and filename
> guesses, then we can turn ffap-lax-url back to nil and everyone will be
> happy.
>
> 1. See thing-at-point--bounds-of-well-formed-url.

I was thinking of the patch below.  There is probably a better solution
out there but I am not sure what other lax URL we will miss out by
setting ffap-lax-url to nil.  It feels like a hack though...

diff --git a/lisp/ffap.el b/lisp/ffap.el
index 530e3da182e..b8da1292d4f 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -152,7 +152,7 @@ ffap-url-unwrap-remote
   :group 'ffap
   :version "24.3")
 
-(defcustom ffap-lax-url t
+(defcustom ffap-lax-url nil
   "If non-nil, allow lax URL matching.
 The default non-nil value might produce false URLs in C++ code
 with symbols like \"std::find\".  On the other hand, setting
@@ -160,7 +160,7 @@ ffap-lax-url
 well-formed, such as \"user@host\" or \"<user@host>\"."
   :type 'boolean
   :group 'ffap
-  :version "25.2")                      ; nil -> t
+  :version "30.1")
 
 (defcustom ffap-ftp-default-user "anonymous"
   "User name in FTP file names generated by `ffap-host-to-filename'.
@@ -609,6 +609,13 @@ ffap-fixup-url
    ((and ffap-url-unwrap-remote (ffap-url-unwrap-remote url)))
    (url)))
 
+(defun ffap-fixup-email (email)
+  "Clean up EMAIL and return it as a mailto: URL."
+  (when (stringp email)
+    (if (string-prefix-p "mailto:" email)
+        email
+      (concat "mailto:" email))))
+
 \f
 ;;; File Name Handling:
 
@@ -1571,6 +1578,7 @@ ffap-guesser
 	   (ffap-fixup-url (or (ffap-url-at-point)
 			       (ffap-gopher-at-point))))
       (ffap-file-at-point)		; may yield url!
+      (ffap-fixup-email (thing-at-point 'email))
       (ffap-fixup-machine (ffap-machine-at-point))))
 
 (defun ffap-prompter (&optional guess suffix)

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Eli Zaretskii]

> Cc: grfz@gmx.de, 67688@debbugs.gnu.org
> From: Visuwesh <visuweshm@gmail.com>
> Date: Tue, 12 Dec 2023 20:41:28 +0530
> 
> -(defcustom ffap-lax-url t
> +(defcustom ffap-lax-url nil

This change of the default requires a NEWS entry.

Thanks.

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Visuwesh]

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

[Tuesday December 12, 2023] Eli Zaretskii wrote:

>> Cc: grfz@gmx.de, 67688@debbugs.gnu.org
>> From: Visuwesh <visuweshm@gmail.com>
>> Date: Tue, 12 Dec 2023 20:41:28 +0530
>> 
>> -(defcustom ffap-lax-url t
>> +(defcustom ffap-lax-url nil
>
> This change of the default requires a NEWS entry.
>
> Thanks.

I was waiting for review from others.  Nevertheless, I have attached a
proper patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-ffap-correctly-guess-remote-file-names-at-point.patch --]
[-- Type: text/x-diff, Size: 2621 bytes --]

From d2775636e8468490ffba50df888f92000ef5286b Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Tue, 12 Dec 2023 21:14:12 +0530
Subject: [PATCH] Make ffap correctly guess remote file names at point

* lisp/ffap.el (ffap-lax-url): Set it to nil so that remote file names
may be matched.  (bug#67688)
(ffap-fixup-email): New function.
(ffap-guesser): Specially handle email addresses now that ffap-lax-url
is nil as user@host fails to be matched as an email address with that
setting.

* etc/NEWS: Announce the new value of the defcustom.
---
 etc/NEWS     |  5 +++++
 lisp/ffap.el | 14 +++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 29f4e5c0b66..d4a15f08eae 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -258,6 +258,11 @@ called in the '--eval' expression, which is useful when those
 arguments contain arbitrary characters that otherwise might require
 elaborate and error-prone escaping (to protect them from the shell).
 
+---
+** ffap-lax-url now defaults to nil.
+Previously, it was set to ffap-lax-url to t but this broke remote file
+name detection.
+
 \f
 * Editing Changes in Emacs 30.1
 
diff --git a/lisp/ffap.el b/lisp/ffap.el
index 530e3da182e..9cea11cf540 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -152,15 +152,15 @@ ffap-url-unwrap-remote
   :group 'ffap
   :version "24.3")
 
-(defcustom ffap-lax-url t
+(defcustom ffap-lax-url nil
   "If non-nil, allow lax URL matching.
 The default non-nil value might produce false URLs in C++ code
 with symbols like \"std::find\".  On the other hand, setting
 this to nil will disable recognition of URLs that are not
-well-formed, such as \"user@host\" or \"<user@host>\"."
+well-formed."
   :type 'boolean
   :group 'ffap
-  :version "25.2")                      ; nil -> t
+  :version "30.1")
 
 (defcustom ffap-ftp-default-user "anonymous"
   "User name in FTP file names generated by `ffap-host-to-filename'.
@@ -609,6 +609,13 @@ ffap-fixup-url
    ((and ffap-url-unwrap-remote (ffap-url-unwrap-remote url)))
    (url)))
 
+(defun ffap-fixup-email (email)
+  "Clean up EMAIL and return it as a mailto: URL."
+  (when (stringp email)
+    (if (string-prefix-p "mailto:" email)
+        email
+      (concat "mailto:" email))))
+
 \f
 ;;; File Name Handling:
 
@@ -1571,6 +1578,7 @@ ffap-guesser
 	   (ffap-fixup-url (or (ffap-url-at-point)
 			       (ffap-gopher-at-point))))
       (ffap-file-at-point)		; may yield url!
+      (ffap-fixup-email (thing-at-point 'email))
       (ffap-fixup-machine (ffap-machine-at-point))))
 
 (defun ffap-prompter (&optional guess suffix)
-- 
2.42.0

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Gregor Zattler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

* Visuwesh <visuweshm@gmail.com> [2023-12-12; 21:15 +0530]:
> I was waiting for review from others.  Nevertheless, I have attached a
> proper patch.

I cannot review the patch but for me it works as
expected after applying the patch.

Thanks, Gregor

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Eli Zaretskii]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: michael.albinus@gmx.de,  grfz@gmx.de,  67688@debbugs.gnu.org
> Date: Tue, 12 Dec 2023 21:15:02 +0530
> 
> [Tuesday December 12, 2023] Eli Zaretskii wrote:
> 
> >> Cc: grfz@gmx.de, 67688@debbugs.gnu.org
> >> From: Visuwesh <visuweshm@gmail.com>
> >> Date: Tue, 12 Dec 2023 20:41:28 +0530
> >> 
> >> -(defcustom ffap-lax-url t
> >> +(defcustom ffap-lax-url nil
> >
> > This change of the default requires a NEWS entry.
> >
> > Thanks.
> 
> I was waiting for review from others.  Nevertheless, I have attached a
> proper patch.

Thanks.  Let's wait a bit longer, but meanwhile, a nit:

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -258,6 +258,11 @@ called in the '--eval' expression, which is useful when those
>  arguments contain arbitrary characters that otherwise might require
>  elaborate and error-prone escaping (to protect them from the shell).
>  
> +---
> +** ffap-lax-url now defaults to nil.
> +Previously, it was set to ffap-lax-url to t but this broke remote file
> +name detection.

Please quote symbols in NEWS 'like this'.

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Visuwesh]

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

[சனி டிசம்பர் 16, 2023] Eli Zaretskii wrote:

>> From: Visuwesh <visuweshm@gmail.com>
>> Cc: michael.albinus@gmx.de,  grfz@gmx.de,  67688@debbugs.gnu.org
>> Date: Tue, 12 Dec 2023 21:15:02 +0530
>> 
>> [Tuesday December 12, 2023] Eli Zaretskii wrote:
>> 
>> >> Cc: grfz@gmx.de, 67688@debbugs.gnu.org
>> >> From: Visuwesh <visuweshm@gmail.com>
>> >> Date: Tue, 12 Dec 2023 20:41:28 +0530
>> >> 
>> >> -(defcustom ffap-lax-url t
>> >> +(defcustom ffap-lax-url nil
>> >
>> > This change of the default requires a NEWS entry.
>> >
>> > Thanks.
>> 
>> I was waiting for review from others.  Nevertheless, I have attached a
>> proper patch.
>
> Thanks.  Let's wait a bit longer, but meanwhile, a nit:
>
>> --- a/etc/NEWS
>> +++ b/etc/NEWS
>> @@ -258,6 +258,11 @@ called in the '--eval' expression, which is useful when those
>>  arguments contain arbitrary characters that otherwise might require
>>  elaborate and error-prone escaping (to protect them from the shell).
>>  
>> +---
>> +** ffap-lax-url now defaults to nil.
>> +Previously, it was set to ffap-lax-url to t but this broke remote file
>> +name detection.
>
> Please quote symbols in NEWS 'like this'.

Oops, fixed in attached patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-ffap-correctly-guess-remote-file-names-at-point.patch --]
[-- Type: text/x-diff, Size: 2625 bytes --]

From 7769b62cd23bf3eb879c34e0c78ed7f367791993 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Tue, 12 Dec 2023 21:14:12 +0530
Subject: [PATCH] Make ffap correctly guess remote file names at point

* lisp/ffap.el (ffap-lax-url): Set it to nil so that remote file names
may be matched.  (bug#67688)
(ffap-fixup-email): New function.
(ffap-guesser): Specially handle email addresses now that ffap-lax-url
is nil as user@host fails to be matched as an email address with that
setting.

* etc/NEWS: Announce the new value of the defcustom.
---
 etc/NEWS     |  5 +++++
 lisp/ffap.el | 14 +++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 29f4e5c0b66..3e527fcd2ad 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -258,6 +258,11 @@ called in the '--eval' expression, which is useful when those
 arguments contain arbitrary characters that otherwise might require
 elaborate and error-prone escaping (to protect them from the shell).
 
+---
+** 'ffap-lax-url' now defaults to nil.
+Previously, it was set to 'ffap-lax-url' to t but this broke remote file
+name detection.
+
 \f
 * Editing Changes in Emacs 30.1
 
diff --git a/lisp/ffap.el b/lisp/ffap.el
index 530e3da182e..9cea11cf540 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -152,15 +152,15 @@ ffap-url-unwrap-remote
   :group 'ffap
   :version "24.3")
 
-(defcustom ffap-lax-url t
+(defcustom ffap-lax-url nil
   "If non-nil, allow lax URL matching.
 The default non-nil value might produce false URLs in C++ code
 with symbols like \"std::find\".  On the other hand, setting
 this to nil will disable recognition of URLs that are not
-well-formed, such as \"user@host\" or \"<user@host>\"."
+well-formed."
   :type 'boolean
   :group 'ffap
-  :version "25.2")                      ; nil -> t
+  :version "30.1")
 
 (defcustom ffap-ftp-default-user "anonymous"
   "User name in FTP file names generated by `ffap-host-to-filename'.
@@ -609,6 +609,13 @@ ffap-fixup-url
    ((and ffap-url-unwrap-remote (ffap-url-unwrap-remote url)))
    (url)))
 
+(defun ffap-fixup-email (email)
+  "Clean up EMAIL and return it as a mailto: URL."
+  (when (stringp email)
+    (if (string-prefix-p "mailto:" email)
+        email
+      (concat "mailto:" email))))
+
 \f
 ;;; File Name Handling:
 
@@ -1571,6 +1578,7 @@ ffap-guesser
 	   (ffap-fixup-url (or (ffap-url-at-point)
 			       (ffap-gopher-at-point))))
       (ffap-file-at-point)		; may yield url!
+      (ffap-fixup-email (thing-at-point 'email))
       (ffap-fixup-machine (ffap-machine-at-point))))
 
 (defun ffap-prompter (&optional guess suffix)
-- 
2.42.0

bug#67688: 30.0.50; ffap prepends buffers default-directory to guessed file name in case of tramp's file name syntax

[Eli Zaretskii]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: michael.albinus@gmx.de,  grfz@gmx.de,  67688@debbugs.gnu.org
> Date: Sat, 16 Dec 2023 20:15:48 +0530
> 
> >> --- a/etc/NEWS
> >> +++ b/etc/NEWS
> >> @@ -258,6 +258,11 @@ called in the '--eval' expression, which is useful when those
> >>  arguments contain arbitrary characters that otherwise might require
> >>  elaborate and error-prone escaping (to protect them from the shell).
> >>  
> >> +---
> >> +** ffap-lax-url now defaults to nil.
> >> +Previously, it was set to ffap-lax-url to t but this broke remote file
> >> +name detection.
> >
> > Please quote symbols in NEWS 'like this'.
> 
> Oops, fixed in attached patch.

Thanks, installed on master, and closing the bug.