Subject: bug#72701: eglot crash when project-files-relative-names t

From: Ship Mints <[email-masked]>
Date: Fri, 23 Aug 2024 10:56:47 -0400

Glad I set project-files-relative-names to t and have been testing. This
side-effect in Eglot is sure to bite other people who are teased by the
NEWS of this new option, despite not being a defcustom.

If it is not intended to be set (yet? ever?), perhaps update the docstring
to indicate this. I do tend to agree with Joao that the API to
project-files probably could have an optional additional argument vs. a
global, especially if the intention is merely to influence a specific
call-site's expected results.

WRT to Eglot using relative as default, in this context it is looking for
directory components and relative names might need to be adorned at least
with "./" so that file-name-directory doesn't fail. Perhaps
project-files-relative-names could be set to a symbol such as
'retain-relative-directory-component.

On Thu, Aug 22, 2024 at 7:22 PM Dmitry Gutov <[email-masked]> wrote:

> Hi!
>
> On 18/08/2024 18:43, Ship Mints wrote:
> > project-files-relative-names was introduced a few months ago, so it's no
> > surprise that there are some side effects when set.
> >
> > This affects the handler for workspace/didChangeWatchedFiles. The result
> > is that dirs-to-watch...
> >
> >           (dirs-to-watch
> >            (delete-dups (mapcar #'file-name-directory
> >                                 (project-files
> >                                  (eglot--project server))))))
> >
> > ...is nil when project-files-relative-names is t, due to
> > file-name-directory failing to parse the directories it was expecting.
> > This results in file-readable-p failing with a nil dir:
>
> You're not really supposed to customize it as an end user (or at least
> not yet) - the caller code either sets it to t to indicate support for
> relative names, or not.
>
> >           (watch-dir (dir)
> >             (when-let ((probe
> >                         (and (file-readable-p dir)
> >
> > One solution which I've tested is simply the defensive:
> >
> >           (project-files-relative-names nil) ; add this
> >           (dirs-to-watch
> >            (delete-dups (mapcar #'file-name-directory
> >                                 (project-files
> >                                  (eglot--project server))))))
>
> ...that said, someday the default value of this var will be flipped, and
> some changes like that might be needed.
>
> The relative version is a bit faster, though, so maybe Eglot could
> switch to relative names too. If it makes sense.
>