GNU bug report logs - #59338
29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59338 in the body.
You can then email your comments to 59338 AT debbugs.gnu.org in the normal way.

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

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

From: Arash Esbati <arash <at> gnu.org>
To: emacs-bugs <bug-gnu-emacs <at> gnu.org>
Subject: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 17 Nov 2022 17:51:01 +0100

Hi all,

the commit 1a2d603bb3 was supposed to fix bug#58790, but it introduces
(or possibly surfaces) another one.  With Emacs (checkout c3b64985aa),
eval'ing the next 2 forms returned an URI:

  (require 'eglot)
  (insert "\n" (format "%s" (eglot--path-to-uri
                             "d:/digestif-test/tikz-test.tex")))
  => file:///d%3A/digestif-test/tikz-test.tex

With Emacs 623db40d, it looks like this:

  (require 'eglot)
  (insert "\n" (format "%s" (eglot--path-to-uri
                             "d:/digestif-test/tikz-test.tex")))
  => d:/digestif-test/tikz-test.tex

I think the underlying problem is with the return value of
`url-generic-parse-url', but I'm not familiar enough with it to make a
judgement.  As the result, digestif-LSP doesn't work with the current
Eglot on Windows -- checkout c3b64985aa does.

This is with:
In GNU Emacs 29.0.50 (build 1, x86_64-w64-mingw32) of 2022-11-17
Repository revision: 623db40dd1cd21623c5cecdc0abbf3ce885f92b1
Repository branch: master
System Description: Microsoft Windows 10 Pro

Best, Arash

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Arash Esbati <arash <at> gnu.org>
Cc: 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 17 Nov 2022 19:06:36 +0200

> From: Arash Esbati <arash <at> gnu.org>
> Date: Thu, 17 Nov 2022 17:51:01 +0100
> 
> the commit 1a2d603bb3 was supposed to fix bug#58790, but it introduces
> (or possibly surfaces) another one.  With Emacs (checkout c3b64985aa),
> eval'ing the next 2 forms returned an URI:
> 
>   (require 'eglot)
>   (insert "\n" (format "%s" (eglot--path-to-uri
>                              "d:/digestif-test/tikz-test.tex")))
>   => file:///d%3A/digestif-test/tikz-test.tex
> 
> With Emacs 623db40d, it looks like this:
> 
>   (require 'eglot)
>   (insert "\n" (format "%s" (eglot--path-to-uri
>                              "d:/digestif-test/tikz-test.tex")))
>   => d:/digestif-test/tikz-test.tex

Both are wrong, right?  The correct URL should AFAIU be

  file:///d:/digestif-test/tikz-test.tex

IOW, the problem is that the URL is being run through url-encode-url,
which doesn't support file:// URLs on Windows properly.

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

From: Augusto Stoffel <arstoffel <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Arash Esbati <arash <at> gnu.org>, 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 17 Nov 2022 12:18:57 -0500

On Thu, 17 Nov 2022 at 19:06, Eli Zaretskii wrote:

> Both are wrong, right?  The correct URL should AFAIU be
>
>   file:///d:/digestif-test/tikz-test.tex
>
> IOW, the problem is that the URL is being run through url-encode-url,
> which doesn't support file:// URLs on Windows properly.

While we are at it, note that

  (url-filename
    (url-generic-parse-url "file:///d:/digestif-test/tikz-test.tex"))
  => "/d:/digestif-test/tikz-test.tex"

is not the right file name under Windows.  Eglot treats this special
case correctly, but every package that deals with file URLs has to
repeat the work.  So there should be a helper function in Emacs for
this.

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

From: Arash Esbati <arash <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 17 Nov 2022 23:33:24 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> Both are wrong, right?  The correct URL should AFAIU be
>
>   file:///d:/digestif-test/tikz-test.tex

Yes, this is also my understanding, after having a closer look.

> IOW, the problem is that the URL is being run through url-encode-url,
> which doesn't support file:// URLs on Windows properly.

As a note, texlab LSP-server still works with this version:

  => d:/digestif-test/tikz-test.tex

and digestif LSP-server accepts this version:

  => file:///d%3A/digestif-test/tikz-test.tex

Best, Arash

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

From: Danny Freeman <danny <at> dfreeman.email>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, bug-gnu-emacs <at> gnu.org,
 Arash Esbati <arash <at> gnu.org>, 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 17 Nov 2022 17:27:40 -0500

Augusto Stoffel <arstoffel <at> gmail.com> writes:

> On Thu, 17 Nov 2022 at 19:06, Eli Zaretskii wrote:
>
>> Both are wrong, right?  The correct URL should AFAIU be
>>
>>   file:///d:/digestif-test/tikz-test.tex
>>
>> IOW, the problem is that the URL is being run through url-encode-url,
>> which doesn't support file:// URLs on Windows properly.
>
> While we are at it, note that
>
>   (url-filename
>     (url-generic-parse-url "file:///d:/digestif-test/tikz-test.tex"))
>   => "/d:/digestif-test/tikz-test.tex"
>
> is not the right file name under Windows.  Eglot treats this special
> case correctly, but every package that deals with file URLs has to
> repeat the work.  So there should be a helper function in Emacs for
> this.

With the original problem: we're now getting a false positive in
eglot when checking for URLs being passed to `eglot--path-to-uri` when
the path is a windows path.

Is there something we can do to detect a windows path and continue
treating it as a path like we were before this change?

```
(defun eglot--path-to-uri (path)
  "URIfy PATH."
  (let ((truepath (file-truename path)))
    (if (and (url-type (url-generic-parse-url truepath))
             (NOT_WINDOWS_PATH truepath) ;; what would this be?
             )
      ;; ... blah blah blah
```

If there is no function available already, it may be enough to check if
the return value of `url-type` is not 1 character. Looking at this list
of what I believe are official URI schemes, all of them have at least
two characters: 
https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml 

-- 
Danny Freeman

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

From: Augusto Stoffel <arstoffel <at> gmail.com>
To: Danny Freeman <danny <at> dfreeman.email>
Cc: Eli Zaretskii <eliz <at> gnu.org>, bug-gnu-emacs <at> gnu.org,
 Arash Esbati <arash <at> gnu.org>, 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 17 Nov 2022 18:12:05 -0500

On Thu, 17 Nov 2022 at 17:27, Danny Freeman wrote:

> ```
> (defun eglot--path-to-uri (path)
>   "URIfy PATH."
>   (let ((truepath (file-truename path)))
>     (if (and (url-type (url-generic-parse-url truepath))
>              (NOT_WINDOWS_PATH truepath) ;; what would this be?
>              )
>       ;; ... blah blah blah
> ```
>
> If there is no function available already, it may be enough to check if
> the return value of `url-type` is not 1 character. Looking at this list
> of what I believe are official URI schemes, all of them have at least
> two characters: 
> https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml 

I think that makes sense.  I find the above logic a bit funny, though.
What do you expect `truepath' to look like if `path' is actually an URI?
Shouldn't `path' be returned unchanged?

I also think that calling `url-generic-parse-url' might be overkill
here.  Based on
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax I would
just test if `path' matches "\\`[A-Za-z][+.0-9A-Za-z-]+:".

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Danny Freeman <danny <at> dfreeman.email>
Cc: arash <at> gnu.org, arstoffel <at> gmail.com, bug-gnu-emacs <at> gnu.org,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Fri, 18 Nov 2022 09:12:11 +0200

> From: Danny Freeman <danny <at> dfreeman.email>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Arash Esbati <arash <at> gnu.org>,
>  59338 <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org
> Date: Thu, 17 Nov 2022 17:27:40 -0500
> 
> Is there something we can do to detect a windows path

You mean, a Windows-style file name?  You can detect that, but it is
easier to test system-type instead: these file names cannot happen on
any system except Windows, and if they do happen, they don't have the
same semantics (i.e., "d:/foo/bar" is NOT an absolute file name on
Posix systems).

Or maybe I don't understand the purpose of the test you have in mind?

> and continue treating it as a path like we were before this change?

I'd advise against such kludges.  If a function wants a file:// URL,
it should receive a valid file:// URL on all systems, and it should be
capable of handling file:// URLs on MS-Windows as well as on Posix
systems.  Likewise with functions which produce file:// URLs.  Letting
local file names into this is a clear path to future bugs, because
many people will not realize this subtlety, and will think they deal
with file:// URLs on all platforms.

> If there is no function available already, it may be enough to check if
> the return value of `url-type` is not 1 character. Looking at this list
> of what I believe are official URI schemes, all of them have at least
> two characters: 
> https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml 

But hosts can have 1-character names (although that is unlikely).

Anyway, I'm against such kludges, especially since we don't need them
here.  We just need to make our functions that handle file:// URLs to
be capable of supporting file:// on MS-Windows.  It is not hard to do,
so let's do that.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: arash <at> gnu.org, bug-gnu-emacs <at> gnu.org, danny <at> dfreeman.email,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Fri, 18 Nov 2022 09:14:18 +0200

> From: Augusto Stoffel <arstoffel <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  Arash Esbati <arash <at> gnu.org>,
>   59338 <at> debbugs.gnu.org,  bug-gnu-emacs <at> gnu.org
> Date: Thu, 17 Nov 2022 18:12:05 -0500
> 
> On Thu, 17 Nov 2022 at 17:27, Danny Freeman wrote:
> 
> I also think that calling `url-generic-parse-url' might be overkill
> here.  Based on
> https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax I would
> just test if `path' matches "\\`[A-Za-z][+.0-9A-Za-z-]+:".

Not sure I understand why this matters in the context of this
discussion.  We need to make eglot--path-to-uri produce valid file://
URL on MS-Windows and on Posix systems, right?  Then why does it
matter how URI schema are defined?  What am I missing?

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

From: Danny Freeman <danny <at> dfreeman.email>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: arash <at> gnu.org, Augusto Stoffel <arstoffel <at> gmail.com>, bug-gnu-emacs <at> gnu.org,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Fri, 18 Nov 2022 08:39:13 -0500

Eli Zaretskii <eliz <at> gnu.org> writes:

>
> Not sure I understand why this matters in the context of this
> discussion.  We need to make eglot--path-to-uri produce valid file://
> URL on MS-Windows and on Posix systems, right?  Then why does it
> matter how URI schema are defined?  What am I missing?

The current eglot--path-to-uri implementation should produce a valid
file:// url unless what it receives is already a URL.

So it could receive something like:

/home/user/project/whatever.c
d:/what/home/is/on/windows/whatever.c

Both of which should be transformed into file:// URLs
OR what it receives may already be a URL like

zipfile:home/user/project.zip::/path/in/zip.c

If it receives a URL, we want to pass it along, and not transform it
into a file:// URL.

If it is a full windows path, we DO want to turn that into a file url.

So how do we detect that is is a windows path, and not a URL already?
That's what I was trying to get at in the other message you replied to.
Just checking the user's current OS is not enough, because this function
could also receive a URL on Windows.

-- 
Danny Freeman

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Danny Freeman <danny <at> dfreeman.email>
Cc: arash <at> gnu.org, arstoffel <at> gmail.com, bug-gnu-emacs <at> gnu.org,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Fri, 18 Nov 2022 16:35:05 +0200

> From: Danny Freeman <danny <at> dfreeman.email>
> Cc: Augusto Stoffel <arstoffel <at> gmail.com>, arash <at> gnu.org,
>  59338 <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org
> Date: Fri, 18 Nov 2022 08:39:13 -0500
> 
> The current eglot--path-to-uri implementation should produce a valid
> file:// url unless what it receives is already a URL.
> 
> So it could receive something like:
> 
> /home/user/project/whatever.c
> d:/what/home/is/on/windows/whatever.c
> 
> Both of which should be transformed into file:// URLs
> OR what it receives may already be a URL like
> 
> zipfile:home/user/project.zip::/path/in/zip.c
> 
> If it receives a URL, we want to pass it along, and not transform it
> into a file:// URL.
> 
> If it is a full windows path, we DO want to turn that into a file url.
> 
> So how do we detect that is is a windows path, and not a URL already?

You test that system-type is windows-nt AND that the
file-name-absolute-p returns non-nil for the argument.

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

From: Danny Freeman <danny <at> dfreeman.email>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: arash <at> gnu.org, arstoffel <at> gmail.com, bug-gnu-emacs <at> gnu.org,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Fri, 18 Nov 2022 09:55:47 -0500

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> If it is a full windows path, we DO want to turn that into a file url.
>> 
>> So how do we detect that is is a windows path, and not a URL already?
>
> You test that system-type is windows-nt AND that the
> file-name-absolute-p returns non-nil for the argument.

That is incredible simple. Maybe something like this patch will work:

[0001-Eglot-don-t-confuse-URLs-and-windows-file-paths.patch (text/x-patch, attachment)]

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

I can confirm it works on linux, but I don't have a windows machine.
Someone else would need to verify it there.

-- 
Danny Freeman

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

From: Arash Esbati <arash <at> gnu.org>
To: Danny Freeman <danny <at> dfreeman.email>
Cc: Eli Zaretskii <eliz <at> gnu.org>, arstoffel <at> gmail.com, bug-gnu-emacs <at> gnu.org,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Fri, 18 Nov 2022 21:01:54 +0100

Danny Freeman <danny <at> dfreeman.email> writes:

> I can confirm it works on linux, but I don't have a windows machine.
> Someone else would need to verify it there.

Many thanks for working on this.  I tried your patch on my Windows box
and this is what I get:

  (require 'eglot)
  (insert "\n" (format "%s" (eglot--path-to-uri
                             "d:/digestif-test/tikz-test.tex")))
  => file:///d%3A/digestif-test/tikz-test.tex

  (insert "\n" (format "%s" (eglot--path-to-uri
                             "d:/digestif-test/tikz test.tex")))
  => file:///d%3A/digestif-test/tikz%20test.tex

As Eli already mentioned, %-escaping the colon seems to be wrong (I
couldn't find a definitive source for this, though), but it seems a
deliberate decision in eglot.el's `eglot--uri-path-allowed-chars', which
also references this GitHub issue[1].  My original problem is solved and
digestif-LSP works on Windows again.  So for now, I suggest to apply
your patch and close this report.

Best, Arash

Footnotes:
[1]  https://github.com/joaotavora/eglot/pull/639

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

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Richard Copley <rcopley <at> gmail.com>
Cc: arash <at> gnu.org, arstoffel <at> gmail.com, Danny Freeman <danny <at> dfreeman.email>,
 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Wed, 23 Nov 2022 12:51:37 +0000

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

I just found out this bug was ongoing.

Eli, if you're proposing to fix url-parse.el to not be fooled by windows
file names, then I support that idea, and I think it's the correct
thing to do.

But.... we still need the eglot.el kludge installed because url-parse.el
is not distributed as an ELPA package and Eglot is. So users of
Emacs < 29 would not receive the fix and would have their
WIndows Eglot broken.

João

On Fri, Nov 18, 2022 at 7:13 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Danny Freeman <danny <at> dfreeman.email>
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, Arash Esbati <arash <at> gnu.org>,
> >  59338 <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org
> > Date: Thu, 17 Nov 2022 17:27:40 -0500
> >
> > Is there something we can do to detect a windows path
>
> You mean, a Windows-style file name?  You can detect that, but it is
> easier to test system-type instead: these file names cannot happen on
> any system except Windows, and if they do happen, they don't have the
> same semantics (i.e., "d:/foo/bar" is NOT an absolute file name on
> Posix systems).
>
> Or maybe I don't understand the purpose of the test you have in mind?
>
> > and continue treating it as a path like we were before this change?
>
> I'd advise against such kludges.  If a function wants a file:// URL,
> it should receive a valid file:// URL on all systems, and it should be
> capable of handling file:// URLs on MS-Windows as well as on Posix
> systems.  Likewise with functions which produce file:// URLs.  Letting
> local file names into this is a clear path to future bugs, because
> many people will not realize this subtlety, and will think they deal
> with file:// URLs on all platforms.
>
> > If there is no function available already, it may be enough to check if
> > the return value of `url-type` is not 1 character. Looking at this list
> > of what I believe are official URI schemes, all of them have at least
> > two characters:
> > https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
>
> But hosts can have 1-character names (although that is unlikely).
>
> Anyway, I'm against such kludges, especially since we don't need them
> here.  We just need to make our functions that handle file:// URLs to
> be capable of supporting file:// on MS-Windows.  It is not hard to do,
> so let's do that.
>
>
>
>

-- 
João Távora

[Message part 2 (text/html, inline)]

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

From: Danny Freeman <danny <at> dfreeman.email>
To: João Távora <joaotavora <at> gmail.com>
Cc: Richard Copley <rcopley <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 arstoffel <at> gmail.com, arash <at> gnu.org, 59338 <at> debbugs.gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 24 Nov 2022 08:42:36 -0500

João Távora <joaotavora <at> gmail.com> writes:

> I just found out this bug was ongoing.
>
> Eli, if you're proposing to fix url-parse.el to not be fooled by windows
> file names, then I support that idea, and I think it's the correct
> thing to do.
>
> But.... we still need the eglot.el kludge installed because url-parse.el
> is not distributed as an ELPA package and Eglot is. So users of
> Emacs < 29 would not receive the fix and would have their
> WIndows Eglot broken.
>
> João
>

Should my patch for eglot be merged then?
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59338#53

-- 
Danny Freeman

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

From: João Távora <joaotavora <at> gmail.com>
To: Danny Freeman <danny <at> dfreeman.email>, 59338-done <at> debbugs.gnu.org
Cc: Richard Copley <rcopley <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 arstoffel <at> gmail.com, arash <at> gnu.org
Subject: Re: bug#59338: 29.0.50; Commit 1a2d603bb3 breaks Eglot on Windows
Date: Thu, 24 Nov 2022 15:27:57 +0000

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

I've just tested it on a Windows machine and pushed it, thanks.

Closing this.

João

On Thu, Nov 24, 2022 at 1:44 PM Danny Freeman <danny <at> dfreeman.email> wrote:

>
> João Távora <joaotavora <at> gmail.com> writes:
>
> > I just found out this bug was ongoing.
> >
> > Eli, if you're proposing to fix url-parse.el to not be fooled by windows
> > file names, then I support that idea, and I think it's the correct
> > thing to do.
> >
> > But.... we still need the eglot.el kludge installed because url-parse.el
> > is not distributed as an ELPA package and Eglot is. So users of
> > Emacs < 29 would not receive the fix and would have their
> > WIndows Eglot broken.
> >
> > João
> >
>
> Should my patch for eglot be merged then?
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59338#53
>
> --
> Danny Freeman
>


-- 
João Távora

[Message part 2 (text/html, inline)]

Previous Next

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