Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

fix: underscore autocompletion produces duplicated text #37

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
mscolnick merged 1 commit into main from ms/fix-prefix-match
Feb 9, 2026
Merged

fix: underscore autocompletion produces duplicated text #37

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
37 changes: 37 additions & 0 deletions src/__tests__/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,43 @@ describe("prefixMatch", () => {
expect(result).toBeInstanceOf(RegExp);
expect("a").toMatch(result as RegExp);
});

it("should match partial prefix for underscore variables", () => {
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test name is misleading: this case is about the user typing beyond the common prefix (e.g., REF vs common prefix RE), not specifically about underscores (the underscore isn't relevant to the assertion). Renaming the test to reflect the "typed text extends beyond common prefix" scenario would make intent clearer.

Suggested change
it("should match partial prefix for underscore variables", () => {
it("should handle typed text extending beyond common prefix", () => {

Copilot uses AI. Check for mistakes.
// When items share a prefix like "RE" but user typed "REF",
// the regex should match "RE" but NOT "REF"
const items: LSP.CompletionItem[] = [
{ label: "REF_VALUE" },
{ label: "RESULT" },
{ label: "RETURN" },
];

const result = prefixMatch(items);
expect(result).toBeInstanceOf(RegExp);

// The common prefix is "RE", so the regex matches "R" and "RE"
expect("R").toMatch(result as RegExp);
expect("RE").toMatch(result as RegExp);
// "REF" does NOT match because it extends beyond the common prefix
// This is the scenario that requires a fallback in the caller
expect("REF").not.toMatch(result as RegExp);
});

it("should handle underscore in common prefix", () => {
// When all items share a prefix including underscore
const items: LSP.CompletionItem[] = [
{ label: "REF_VALUE" },
{ label: "REF_COUNT" },
];

const result = prefixMatch(items);
expect(result).toBeInstanceOf(RegExp);

// Common prefix is "REF_" so all substrings match
expect("R").toMatch(result as RegExp);
expect("RE").toMatch(result as RegExp);
expect("REF").toMatch(result as RegExp);
expect("REF_").toMatch(result as RegExp);
});
});

describe("renderMarkdown", () => {
Expand Down
8 changes: 5 additions & 3 deletions src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,11 @@ export class LanguageServerPlugin implements PluginValue {
const match = prefixMatch(items);

const token = match
? context.matchBefore(match)
: // Fallback to matching any character
context.matchBefore(/[a-zA-Z0-9]+/);
? // Try prefix-based match, then fall back to general word match
(context.matchBefore(match) ??
context.matchBefore(/[a-zA-Z0-9_]+/))
: // Fallback to matching any word character
Comment on lines 309 to +313
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation inside the nullish-coalescing expression is inconsistent with the surrounding formatting (the second context.matchBefore line is less indented). Running pnpm run lint (Biome) or adjusting indentation will keep formatting consistent and avoid churn in future edits.

Copilot uses AI. Check for mistakes.
context.matchBefore(/[a-zA-Z0-9_]+/);
Comment on lines 309 to +314
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new fallback logic in requestCompletion changes how from is computed when context.matchBefore(match) returns null, but there's no test exercising this scenario. Consider adding a unit test in src/__tests__/language-server-plugin.test.ts that mocks context.matchBefore to return null for the prefix regex and a token for the fallback regex, then asserts the returned CompletionResult.from uses the fallback token's from (preventing duplicated insertions like REFREF_VALUE).

Copilot uses AI. Check for mistakes.
let { pos } = context;

const sortedItems = sortCompletionItems(
Expand Down
Loading