Skip to content

Remove git scheme from document selector #3540

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

Merged
merged 1 commit into from
Jun 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 23 additions & 28 deletions vscode/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ function collectClientOptions(

const features: EnabledFeatures = configuration.get("enabledFeatures")!;
const enabledFeatures = Object.keys(features).filter((key) => features[key]);
const supportedSchemes = ["file", "git"];

const fsPath = workspaceFolder.uri.fsPath.replace(/\/$/, "");

Expand All @@ -191,9 +190,7 @@ function collectClientOptions(
// 3. Default gems
let documentSelector: DocumentSelector = SUPPORTED_LANGUAGE_IDS.flatMap(
(language) => {
return supportedSchemes.map((scheme) => {
return { scheme, language, pattern: `${fsPath}/**/*` };
});
return { scheme: "file", language, pattern: `${fsPath}/**/*` };
},
);

Expand All @@ -209,34 +206,32 @@ function collectClientOptions(
}

ruby.gemPath.forEach((gemPath) => {
supportedSchemes.forEach((scheme) => {
// On Windows, gem paths may be using backslashes, but those are not valid as a glob pattern. We need to ensure
// that we're using forward slashes for the document selectors
const pathAsGlobPattern = gemPath.replace(/\\/g, "/");
// On Windows, gem paths may be using backslashes, but those are not valid as a glob pattern. We need to ensure
// that we're using forward slashes for the document selectors
const pathAsGlobPattern = gemPath.replace(/\\/g, "/");

documentSelector.push({
scheme: "file",
language: "ruby",
pattern: `${pathAsGlobPattern}/**/*`,
});

// Because of how default gems are installed, the gemPath location is actually not exactly where the files are
// located. With the regex, we are correcting the default gem path from this (where the files are not located)
// /opt/rubies/3.3.1/lib/ruby/gems/3.3.0
//
// to this (where the files are actually stored)
// /opt/rubies/3.3.1/lib/ruby/3.3.0
//
// Notice that we still need to add the regular path to the selector because some version managers will install
// gems under the non-corrected path
if (/lib\/ruby\/gems\/(?=\d)/.test(pathAsGlobPattern)) {
documentSelector.push({
scheme,
scheme: "file",
language: "ruby",
pattern: `${pathAsGlobPattern}/**/*`,
pattern: `${pathAsGlobPattern.replace(/lib\/ruby\/gems\/(?=\d)/, "lib/ruby/")}/**/*`,
});

// Because of how default gems are installed, the gemPath location is actually not exactly where the files are
// located. With the regex, we are correcting the default gem path from this (where the files are not located)
// /opt/rubies/3.3.1/lib/ruby/gems/3.3.0
//
// to this (where the files are actually stored)
// /opt/rubies/3.3.1/lib/ruby/3.3.0
//
// Notice that we still need to add the regular path to the selector because some version managers will install
// gems under the non-corrected path
if (/lib\/ruby\/gems\/(?=\d)/.test(pathAsGlobPattern)) {
documentSelector.push({
scheme,
language: "ruby",
pattern: `${pathAsGlobPattern.replace(/lib\/ruby\/gems\/(?=\d)/, "lib/ruby/")}/**/*`,
});
}
});
}
});

// This is a temporary solution as an escape hatch for users who cannot upgrade the `ruby-lsp` gem to a version that
Expand Down
14 changes: 1 addition & 13 deletions vscode/src/test/suite/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ suite("Client", () => {
test("document selectors match default gems and bundled gems appropriately", () => {
const selector = client.clientOptions
.documentSelector! as TextDocumentFilter[];
assert.strictEqual(selector.length, 10);
assert.strictEqual(selector.length, 5);

// We don't care about the order of the document filters, just that they are present. This assertion helper is just
// a convenience to search the registered filters
Expand All @@ -641,26 +641,14 @@ suite("Client", () => {
};

assertSelector("ruby", `${workspaceUri.fsPath}/**/*`, "file");
assertSelector("ruby", `${workspaceUri.fsPath}/**/*`, "git");
assertSelector("erb", `${workspaceUri.fsPath}/**/*`, "file");
assertSelector("erb", `${workspaceUri.fsPath}/**/*`, "git");

assertSelector(
"ruby",
new RegExp(`ruby\\/\\d\\.\\d\\.\\d\\/\\*\\*\\/\\*`),
"file",
);
assertSelector(
"ruby",
new RegExp(`ruby\\/\\d\\.\\d\\.\\d\\/\\*\\*\\/\\*`),
"git",
);

assertSelector("ruby", /lib\/ruby\/gems\/\d\.\d\.\d\/\*\*\/\*/, "file");
assertSelector("ruby", /lib\/ruby\/gems\/\d\.\d\.\d\/\*\*\/\*/, "git");

assertSelector("ruby", /lib\/ruby\/\d\.\d\.\d\/\*\*\/\*/, "file");
assertSelector("ruby", /lib\/ruby\/\d\.\d\.\d\/\*\*\/\*/, "git");
});

test("requests for non existing documents do not crash the server", async () => {
Expand Down