Repository navigation
Conversation
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options only cover the documents a server was opened with, as they apply to the documents the client selects for the server, but `didSave` registrations may select any document of the worktree. This is what rust-analyzer relies on: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Until zed-industries#65252 makes Zed match `pattern`, such pattern-only filters match every document, so rust-analyzer keeps receiving the saves of every file as it does today. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. This removes the broadcast of every save to every server, which rust-analyzer does not need: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2749ded to
c978a89
Compare
selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c978a89 to
3dceafb
Compare
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. Registrations without a selector stand for the same documents as the static options. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. Registrations without a selector stand for the same documents as the static options. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. Registrations without a selector stand for the same documents as the static options. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Thanks, this one seems relatively ready, left 2 notes.
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. Registrations without a selector stand for the same documents as the static options. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zed-industries#59243 made dynamically registered capabilities honor the `language` and `scheme` of their document selectors, but left `pattern` matching fail-open, so a registration such as `{ "language": "toml", "pattern": "**/Project.toml" }` still applied to every TOML document the server had opened, and a pattern-only filter applied to every document. `pattern` is now matched against the document's path, for both plain glob patterns and relative patterns resolved against their base URI, and Zed advertises the `textDocument.filters.relativePatternSupport` client capability. As in the LSP glob syntax, `*` and `?` do not match path separators. Documents without a file system path never match a `pattern` filter. Contexts that stand for a whole language, like the semantic token legend lookup, and guest buffers whose path is unknown keep not evaluating patterns. LSP 3.18 changed `TextDocumentFilter.pattern` from a string to a `GlobPattern`, so that it can also be a relative pattern. lsp-types is updated to the revision with zed-industries/lsp-types#16, which types `DocumentFilter.pattern` accordingly and adds the `textDocument.filters` client capability. Registrations whose patterns are not valid globs are rejected when they are registered, so matching never has to guess what they mean. Release Notes: - Fixed language servers' dynamically registered capabilities applying to files outside of the `pattern` of their document selector. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Remote clients matched the path of a document, and the base URI of a relative pattern, with their own platform's path rules. A guest on macOS or Linux saw `C:\repo\a.rs` from a Windows host as a single path component, so a selector such as `C:/repo/a.rs` did not match it, and the guest gave up on dynamically registered capabilities, such as a completion provider, before asking the host. Document paths are now normalized to `/` separators according to the path style of their worktree, which is the host's on remote clients, and relative pattern base URIs are converted with the same path style, comparing the base case-insensitively on Windows, where URIs often carry lowercase drive letters. Release Notes: - N/A Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`globset` matches globs against bytes, so `?` and character classes in
document filter patterns could not match a non-ASCII character: a valid
registration for `**/?.rs` did not apply to `é.rs`.
Patterns are now compiled into regular expressions over characters,
following the glob semantics of VS Code that language servers are
written against: `*` and `?` match within a path segment, a `**`
segment matches any number of segments, `{a,b}` matches either
sub-pattern, and `[a-z]` or `[!a-z]` matches a character in or out of a
range. Patterns with an unclosed `{` or `[` are still rejected when
they are registered.
Release Notes:
- N/A
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3dceafb to
df954d9
Compare
|
Q. This PR’s relative patterns can make older collaboration guests fail to deserialize the entire LSP capabilities payload. Should we preserve wire compatibility here, add a |
zed-industries#59243 made dynamically registered capabilities honor their document selectors, but `textDocument/didChange` and `textDocument/didSave` registrations still overwrote the server-wide sync options and ignored their selectors, while `didSave` was sent for every saved file of the worktree to every server whose options asked for saves. These registrations are now kept per registration, with their selectors. The sync kind of a document's changes and whether its saves are notified, with or without text, come from the latest registration matching the document, falling back to the static options. The merged capabilities keep mirroring the latest registrations, and unregistering now restores the static options instead of clearing them. A `didSave` registration without options is also honored now, instead of ignored. Static save options now only cover the documents a server was opened with, as they apply to the documents the client selects for the server, while `didSave` registrations may select any document of the worktree. Registrations without a selector stand for the same documents as the static options. This removes the broadcast of every save to every server, which rust-analyzer no longer needs to be functional in Zed: with dynamic registration it declares no static save options, and registers `didSave` for `**/*.rs` and `**/Cargo.toml` to reload its workspace when a manifest is saved. Manifest saves keep reaching it either way: while `pattern` matching remains fail-open, its filters match every document, and once zed-industries#65252 lands they select exactly these files. Documents without a language no longer match every registration, and only match filters without a `language`. Release Notes: - Fixed language servers receiving change and save notifications that do not follow their dynamic registrations, including save notifications for unrelated files of the project. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Let's assume we can ignore the collab compatibility issues for now, and focus on the PR implementation, as there are things to tackle before.
| return false; | ||
| }; | ||
| let Some(base_uri) = base_uri else { | ||
| return matcher.is_match(path); |
There was a problem hiding this comment.
Drive-letter case is still handled only for relative bases.
With the PR’s exact matcher, the selector c:/repo/a.rs does not match the Windows document C:\repo\a.rs, since candidate normalization preserves C: and this branch uses the case-sensitive regex directly.
VS Code instead matches against Uri.fsPath, whose drive-prefix conversion lowercases the drive letter.
For a dynamically registered provider with no static counterpart, the registration is therefore still excluded by capability filtering.
We seem to need a consistent drive-prefix representation for absolute matching too, while preserving the rest of the pattern’s case sensitivity.
| let Some(path_prefix) = path.get(..base_path.len()) else { | ||
| return false; | ||
| }; | ||
| let base_path_matches = match path_style { | ||
| // Windows paths are case-insensitive, and URIs often carry lowercase drive letters. | ||
| PathStyle::Windows => path_prefix.eq_ignore_ascii_case(base_path), |
There was a problem hiding this comment.
The Windows base comparison is still ASCII-only.
With the PR’s exact matcher:
| Base URI | Pattern | Document | Expected | Actual |
|---|---|---|---|---|
file:///C:/%C3%89quipe |
a.rs |
C:\équipe\a.rs |
Match | No match |
eq_ignore_ascii_case cannot equate É and é, whereas VS Code’s comparison handles this non-ASCII case difference.
This excludes an otherwise applicable dynamic registration through capability filtering.
Use a Unicode-aware prefix comparison and derive the remaining path from the matched candidate prefix rather than the base’s UTF-8 byte length.
Case-equivalent strings need not have equal UTF-8 lengths; for example, K and k occupy three bytes and one byte respectively.
| let is_last_segment = index + 1 == segments.len(); | ||
| if segment == GLOBSTAR { | ||
| if !previous_segment_was_globstar { | ||
| // Any number of segments, including the separators around them, and a last | ||
| // segment when nothing follows. | ||
| regex.push_str(if is_last_segment { | ||
| "(?:/|[^/]+/|/[^/]+)*?" | ||
| } else { | ||
| "(?:/|[^/]+/)*?" | ||
| }); | ||
| } | ||
| previous_segment_was_globstar = true; |
There was a problem hiding this comment.
Consecutive trailing globstars lose the final filename match:
| Pattern | Document | Expected by LSP | Actual |
|---|---|---|---|
/root/** |
/root/a.rs |
Match | Match |
/root/**/** |
/root/a.rs |
Match | No match |
The first ** is compiled as nonterminal, then the terminal ** is skipped as a duplicate, leaving a regex that requires a trailing separator.
The LSP definition allows each ** to match zero or more path segments, so the second selector should not lose applicability for this file.
This also reproduces in VS Code’s parser, so it is a spec-conformance gap rather than a VS Code compatibility regression.
If LSP semantics are the target, collapse consecutive globstars before deciding which one is terminal; if exact VS Code behavior is the target, this needs to be an explicit compatibility exception.
| Some('-') => class.push('-'), | ||
| Some('!' | '^') if class.is_empty() => class.push('^'), | ||
| // Classes never match path separators. | ||
| Some('/') => {} | ||
| Some(char) => { | ||
| class.push_str(®ex::escape(char.encode_utf8(&mut [0; 4]))) | ||
| } | ||
| None => anyhow::bail!("unclosed `[` in glob pattern {glob:?}"), | ||
| } | ||
| } | ||
| regex.push_str(&format!("[{class}]")); |
There was a problem hiding this comment.
Removing a literal / from the class does not prevent a negated class or a range from consuming a separator.
With the PR’s exact matcher:
| Pattern | Document | Expected by LSP | Actual |
|---|---|---|---|
**/a[!x]b.rs |
/root/a/b.rs |
No match | Match |
**/a[.-0]b.rs |
/root/a/b.rs |
No match | Match |
The emitted [^x] and [\.-0] both match /, allowing the registration to apply across a directory boundary even though LSP character classes match within a path segment.
VS Code’s parser has the same result here, so this is another spec-versus-compatibility decision rather than a divergence from VS Code.
For LSP semantics, intersect the completed class with the non-separator character set instead of only removing literal separators while parsing it.
Summary
Follow-up to #59243, which made dynamically registered capabilities honor the
languageandschemeof their document selectors, but leftpatternmatching fail-open. A registration such as{ "language": "toml", "pattern": "**/Project.toml" }therefore still applied to every TOML document the server had opened, and a pattern-only filter applied to every document.This matches
patternagainst the document's path:*and?do not match path separators.textDocument.filters.relativePatternSupportclient capability.patternfilter. Contexts that stand for a whole language, like the semantic token legend lookup, and guest buffers whose path is unknown keep not evaluating patterns.LSP 3.18 changed
TextDocumentFilter.patternfrom a string to aGlobPattern, so that it can also be a relative pattern.This PR depends on zed-industries/lsp-types#16, which updates the type and adds the client capability. I'd like to merge this together with that upstream change:Cargo.tomlcurrently points at the commit of zed-industries/lsp-types#16 on my fork, and I will switch it to thezed-industries/lsp-typesrevision once that PR is merged.Update: zed-industries/lsp-types#16 has been merged, and
Cargo.tomlnow points at itszed-industries/lsp-typesrevision (zed-industries/lsp-types@82d7fc6).Testing
test_dynamic_formatting_registration_with_pattern_only_selector, which asserted the fail-open behavior left by lsp: Honor dynamic LSP document selectors #59243.Self-review
Release Notes:
patternof their document selector.