mirror of
https://github.com/zed-industries/zed.git
synced 2026-10-19 11:04:06 +00:00
snippets: Strip path separators from language snippet file names (#61421)
Closes #59620 ## Problem A language whose name contains a `/`, such as a custom `PL/X` extension (`lsp_id` → `pl/x`), broke snippets end to end: - The `snippets: configure snippets` action wrote the file to `~/.config/zed/snippets/pl/x.json`, i.e. inside a `pl/` subdirectory. - The snippet scanner reads `snippets/` non-recursively and skips directories, so that file was never loaded and the snippet could never be used. - The completion lookup keyed off the raw `lsp_id` (`pl/x`), which wouldn't have matched the file-stem key even if the file had been scanned. So Zed's own UI created a snippet file it could never read back. ## Fix Add `LanguageName::snippet_scope_id()` (the `lsp_id` with `/` and `\` removed) and use it everywhere a language maps to its snippet file name or lookup key: - the Configure Snippets writer and its "already configured" label, and - the two completion lookups in `editor`. `PL/X` now maps to a flat `plx.json`, as suggested in the issue. The `editor::InsertSnippet` action is intentionally left unchanged: its `language` field is documented to be the snippet file name stem, which is already separator-free. Note: files created under the old behavior (nested `foo/bar.json`) aren't migrated; re-running Configure Snippets writes the corrected flat file. ## Testing - Added a `language_core` unit test asserting `snippet_scope_id()` strips `/` and `\` (e.g. `PL/X` → `plx`). - `cargo test -p language_core` passes. - `./script/clippy -p language_core -p language` passes. - Docs Prettier passes. Release Notes: - Fixed snippets being unusable for languages whose name contains a `/` character
This commit is contained in:
parent
6297c88f42
commit
c3422b97a9
5 changed files with 43 additions and 4 deletions
|
|
@ -1140,7 +1140,7 @@ fn has_strong_snippet_prefix_match(
|
|||
|
||||
languages.iter().any(|language| {
|
||||
snippet_store
|
||||
.snippets_for(Some(language.lsp_id()), cx)
|
||||
.snippets_for(Some(language.snippet_scope_id()), cx)
|
||||
.iter()
|
||||
.flat_map(|snippet| snippet.prefix.iter())
|
||||
.flat_map(|prefix| snippet_candidate_suffixes(prefix, &is_word_char))
|
||||
|
|
@ -1161,7 +1161,7 @@ fn snippet_completions(
|
|||
let scopes: Vec<_> = languages
|
||||
.iter()
|
||||
.filter_map(|language| {
|
||||
let language_name = language.lsp_id();
|
||||
let language_name = language.snippet_scope_id();
|
||||
let snippets = snippet_store.snippets_for(Some(language_name), cx);
|
||||
|
||||
if snippets.is_empty() {
|
||||
|
|
|
|||
|
|
@ -1165,6 +1165,10 @@ impl Language {
|
|||
self.config.name.lsp_id()
|
||||
}
|
||||
|
||||
pub fn snippet_scope_id(&self) -> String {
|
||||
self.config.name.snippet_scope_id()
|
||||
}
|
||||
|
||||
pub fn prettier_parser_name(&self) -> Option<&str> {
|
||||
self.config.prettier_parser_name.as_deref()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -51,6 +51,16 @@ impl LanguageName {
|
|||
language_name => language_name.to_lowercase(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Identifier used to name and look up this language's snippet file.
|
||||
///
|
||||
/// Path separators are stripped from the `lsp_id` because the snippet file
|
||||
/// is stored as a flat file directly under the snippets directory; a `/`
|
||||
/// (or `\`) would otherwise place it in a subdirectory that is never
|
||||
/// scanned, making the snippet impossible to use.
|
||||
pub fn snippet_scope_id(&self) -> String {
|
||||
self.lsp_id().replace(['/', '\\'], "")
|
||||
}
|
||||
}
|
||||
|
||||
impl From<LanguageName> for SharedString {
|
||||
|
|
@ -107,3 +117,26 @@ impl From<LanguageName> for String {
|
|||
Self::from(value)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn snippet_scope_id_strips_path_separators() {
|
||||
// A `/` or `\` in the language name would route the snippet file into a
|
||||
// subdirectory that is never scanned (see issue #59620).
|
||||
assert_eq!(LanguageName::new("PL/X").snippet_scope_id(), "plx");
|
||||
assert_eq!(LanguageName::new("a\\b").snippet_scope_id(), "ab");
|
||||
// Names without separators are unchanged beyond `lsp_id`'s lowercasing.
|
||||
assert_eq!(LanguageName::new("Rust").snippet_scope_id(), "rust");
|
||||
assert_eq!(
|
||||
LanguageName::new("Shell Script").snippet_scope_id(),
|
||||
"shell script"
|
||||
);
|
||||
assert_eq!(
|
||||
LanguageName::new("Plain Text").snippet_scope_id(),
|
||||
"plaintext"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -220,7 +220,7 @@ impl PickerDelegate for ScopeSelectorDelegate {
|
|||
cx.spawn_in(window, async move |_, cx| {
|
||||
let scope_file_name = ScopeFileName(match scope_name.to_lowercase().as_str() {
|
||||
GLOBAL_SCOPE_NAME => Cow::Borrowed(GLOBAL_SCOPE_FILE_NAME),
|
||||
_ => Cow::Owned(language.await?.lsp_id()),
|
||||
_ => Cow::Owned(language.await?.snippet_scope_id()),
|
||||
});
|
||||
|
||||
workspace.update_in(cx, |workspace, window, cx| {
|
||||
|
|
@ -322,7 +322,7 @@ impl PickerDelegate for ScopeSelectorDelegate {
|
|||
let name_label = mat.string.clone();
|
||||
|
||||
let scope_name = ScopeName(Cow::Owned(
|
||||
LanguageName::new(&self.candidates[mat.candidate_id].string).lsp_id(),
|
||||
LanguageName::new(&self.candidates[mat.candidate_id].string).snippet_scope_id(),
|
||||
));
|
||||
let file_label = if self.existing_scopes.contains(&scope_name) {
|
||||
Some(ScopeFileName::from(scope_name).with_extension())
|
||||
|
|
|
|||
|
|
@ -39,6 +39,8 @@ The scope is determined by the language name in lowercase e.g. `python.json` for
|
|||
|
||||
To create JSX snippets you have to use `javascript.json` snippets file, instead of `jsx.json`, but this does not apply to TSX and TypeScript which follow the above rule.
|
||||
|
||||
Path separators (`/` and `\`) are removed from the file name, so a language named `PL/X` uses `plx.json`.
|
||||
|
||||
## Known Limitations
|
||||
|
||||
- Only the first prefix is used when a list of prefixes is passed in.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue