Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How do you match a glob pattern at the top-level? #490

Closed
bzoracler opened this issue Aug 18, 2024 · 6 comments
Closed

How do you match a glob pattern at the top-level? #490

bzoracler opened this issue Aug 18, 2024 · 6 comments

Comments

@bzoracler
Copy link

bzoracler commented Aug 18, 2024

I'm not sure if I'm configuring pygls wrong, or if it's the client's issue, or if this is a bug.

I followed #376 to dynamically register the feature workspace/didChangeWatchedFiles, with the watched file pattern **/*.py, which worked fine; 0-or-more nested directories when Python files are added/deleted/changed trigger the notification workspace/didChangeWatchedFiles.

Now, I'm trying to restrict the pattern to files at the top-level. Following the LSP specification, I assume this would look like (following a typescript file extension)

*.ts

but this isn't working ( workspace/didChangeWatchedFiles is not being triggered when adding/deleting/changing .ts files).

I've tried the following:

  • *.ts
  • ./*.ts
  • Using both of the above, and using lsprotocol.types.RelativePattern to construct a pattern, using the current working directory's absolute path to create the argument to the base_uri parameter RelativePattern(base_uri=)

If any of this makes any difference:

  • I'm using Windows
  • The absolute path to the project workspace root is in the PYTHONPATH environment variable;
  • I'm launching the language server at the project workspace root, from a __main__.py script located as <project workspace root>/language_server/__main__.py;
  • I'm not changing the current working directory at all, so cwd is the project workspace root.
@tombh
Copy link
Collaborator

tombh commented Aug 18, 2024

I'm afraid I'm not very familiar with this area, but my first thought is that, as you suggest, this is more of a client issue. Which editor are you using?

@bzoracler
Copy link
Author

bzoracler commented Aug 18, 2024

I've tested this on 2 clients:

The dummy server implementation I'm using:

from pygls.server import LanguageServer
from lsprotocol import types

server = LanguageServer("example-server", "v0.1")

@server.feature(types.INITIALIZED)
def init(ls, params):
        ls.register_capability(types.RegistrationParams(
        registrations = [
            types.Registration(
                id = "matcher-test",
                method = types.WORKSPACE_DID_CHANGE_WATCHED_FILES,
                register_options = types.DidChangeWatchedFilesRegistrationOptions(watchers = [
                    types.FileSystemWatcher(glob_pattern = "*.ts", kind = types.WatchKind.Create | types.WatchKind.Change | types.WatchKind.Delete),
                    types.FileSystemWatcher(glob_pattern = "./*.js", kind = types.WatchKind.Create | types.WatchKind.Change | types.WatchKind.Delete),
                    types.FileSystemWatcher(glob_pattern = "**.ini", kind = types.WatchKind.Create | types.WatchKind.Change | types.WatchKind.Delete),
                    types.FileSystemWatcher(glob_pattern = "**/*.cfg", kind = types.WatchKind.Create | types.WatchKind.Change | types.WatchKind.Delete),
                ])
            )
        ]
    ))

if __name__ == "__main__":
    server.start_io()

In this example, only .cfg additions/changes/deletions trigger the notification workspace/didChangeWatchedFiles (but this triggers it at any nesting level from the root downwards). I can't get additions/changes/deletions of .ini, .ts, and .js to trigger anything at the root.

Given that it doesn't work in 2 different clients, I'm inclined to think that it's something on the server side.

@bzoracler
Copy link
Author

bzoracler commented Aug 23, 2024

The LSP specifications for workspace/didChangeWatchedFiles aren't very clear, but in the two clients I tested (pygls's VSCode extension and LSP4IJ), the glob patterns match absolutely, not relatively.

Therefore, providing a relative glob pattern for the top level, such as *.py, does not work. In contrast, providing an absolute pattern does. If the workspace project consisted solely of a setup.py laid out like the following,

  1. /home/<Name>/projects/my-project/<root>/setup.py (non-Windows)
  2. C:\Users\<Name>\projects\my-project\<root>\setup.py (Windows)

then passing an absolute glob pattern, with some path normalisation handled somewhere between the server and client, will work. The pygls VSCode extension should be able to pick up setup.py using the following patterns, without looking into workspace subdirectories:

  1. /home/<Name>/projects/my-project/<root>/*.py (Non-Windows - no changes)
  2. c:/Users/<Name>/projects/my-project/<root>/*.py (Windows - drive letter lower-cased, path separators converted to POSIX-style)

@tombh
Copy link
Collaborator

tombh commented Aug 24, 2024

So, it is possible to achieve non-recursive, top-level file change notifications by using an absolute path (as opposed to a relative path)? And that's why you closed the issue?

Let me just summarise my understanding of using the glob_pattern argument, to see if we agree:

  1. Providing the traditional double asterisk wildcard path, **/*.py, does work as expected for recursive paths relative to the workspace.
  2. Providing an absolute path with a file-specific wildcard, /home/tombh/projects/foo/*.py, works as expected for non-recursive paths.
  3. Providing a plain file-specific wildcard path, *.py, does not work at all. My intuition for this is that, even without RelativePattern(), it should default to a non-recursive, workspace-root pattern. Therefore, what you were expecting to begin with, and why you made this issue. This could be because of ambiguity in the LSP spec (which we can address). Or at the very least Pygls can justifiably take advantage of the ambiguity and offer the sane default, therefore automatically prepending the workspace root so that clients at least return something rather than nothing.
  4. The above point 3, doesn't even work with RelativePattern(base_uri="/home/tombh/projects/foo"). I feel like this must be a bug in Pygls.

@bzoracler
Copy link
Author

I agree with most of your summary - except whether pygls should provide a default for patterns like *.py in point 3, and whether point 4. is a bug. I didn't yet inspect how the clients interpret the data provided by pygls with a lsprotocol.types.RelativePattern (or even if they use it at all).


I closed this issue because I realised that you were right in it being a client issue, and I'm not sure if there's anything actionable in pygls. Absolute paths like /home/tombh/projects/foo/*.py are likely working because of an implementation detail by the client, not something described in the language server specification:

  • The clients I've tested probably use the same logic, which is taking a glob pattern and matching it against the absolute path. Therefore, /home/tombh/projects/foo/*.py will work, as will **/tombh/projects/foo/*.py, and **/*.py. The fact that the first two patterns are "working" are unintuitive and certainly not described by the spec.
  • The comment here (April 2023) says that watching only a specific folder in the workspace isn't supported until the introduction of a relative pattern. Since RelativePattern was only introduced in 3.17, maybe most clients won't support it yet.
  • pygls implicitly prepending the workspace root to a pattern *.py would only work if (1) the client matches globs absolutely, and (2) the workspace is single-root. But the server should not know anything about the client, and I'm not sure how to interpret *.py in a multi-root workspace (should it match the top-level across all workspace roots? Or should it return nothing?).

@tombh
Copy link
Collaborator

tombh commented Sep 14, 2024

Sorry for the late reply. This thread is a good insight and summary of the issue, so I'm glad you brought it up.

So let's assume that there's nothing that Pygls can do to help at the moment. But we can revisit if needs be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants