Conversation
There was a problem hiding this comment.
Pull request overview
This work-in-progress PR adds a new getSuggestionsLikeVariable function that enables autocomplete suggestions based on partial variable name matching, improving the developer experience when working with CSS variables. The implementation includes refactoring of the existing suggestion sorting logic and updates to word boundary detection.
Key Changes:
- Introduced
getSuggestionsLikeVariableto filter suggestions by variable name substring matching - Refactored suggestion sorting logic into a reusable
sortSuggestionshelper function - Enhanced
getCurrentWordutility with bounds checking and improved delimiter handling
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/utils/get-current-word.ts |
Added offset bounds validation, extracted delimiters to a constant, and fixed right pointer initialization for more accurate word extraction |
src/suggestions.ts |
Added new getSuggestionsLikeVariable function for variable name-based filtering and extracted sorting logic into sortSuggestions helper |
src/suggestions.test.ts |
Added test case for the new getSuggestionsLikeVariable function and a TODO test for outline property suggestions |
src/language-server.ts |
Integrated getSuggestionsLikeVariable to provide context-aware suggestions when the cursor is on a CSS variable, refactored value extraction logic |
Comments suppressed due to low confidence (1)
src/utils/get-current-word.ts:26
- Missing test coverage for
getCurrentWordfunction. The function has been significantly modified (added bounds checking, changed delimiter handling, and fixed right pointer initialization), but there are no tests to verify these changes work correctly. Consider adding tests to cover edge cases like offset at boundaries, empty text, and various delimiter scenarios.
export function getCurrentWord(document: TextDocument, offset: number): string {
const text = document.getText()
const delimiters = ' \t\n\r":{[()]},*>+'
// Ensure offset is within bounds
if (offset < 0 || offset > text.length) {
return ''
}
let left = offset - 1
let right = offset
// Move left to find start of word
while (left >= 0 && delimiters.indexOf(text.charAt(left)) === -1) {
left--
}
// Move right to find end of word
while (right < text.length && delimiters.indexOf(text.charAt(right)) === -1) {
right++
}
return text.substring(left + 1, right)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const document = documents.get(params.textDocument.uri) | ||
| const offset = document.offsetAt(params.position) | ||
| const currentWord = getCurrentWord(document, offset) |
There was a problem hiding this comment.
Redundant variable assignment. The doc variable is already retrieved at line 50. Consider reusing doc instead of fetching the document again with const document = documents.get(params.textDocument.uri).
| const document = documents.get(params.textDocument.uri) | |
| const offset = document.offsetAt(params.position) | |
| const currentWord = getCurrentWord(document, offset) | |
| const offset = doc.offsetAt(params.position) | |
| const currentWord = getCurrentWord(doc, offset) |
| }) | ||
|
|
||
| it('returns suggestions for variable part: --fgColor', () => { | ||
| const suggestions = getSuggestionsLikeVariable('--fgColor') |
There was a problem hiding this comment.
Inconsistent API usage in test. The test passes '--fgColor' (with dashes) to getSuggestionsLikeVariable, but in the actual implementation at language-server.ts:81, the '--' prefix is stripped before calling this function (currentWord.replace('--', '')). The test should either pass 'fgColor' without dashes to match the actual usage, or the function should be documented to accept both formats.
| const suggestions = getSuggestionsLikeVariable('--fgColor') | |
| const suggestions = getSuggestionsLikeVariable('fgColor') |
| export const getSuggestionsLikeVariable = (variableQuery: string): SuggestionWithSortText[] => { | ||
| const allVariables = flatten(Object.values(propertiesMap)) | ||
| const suggestedVariables: Suggestion[] = allVariables.filter(variable => variable.name.includes(variableQuery)) |
There was a problem hiding this comment.
Performance concern: flatten(Object.values(propertiesMap)) is executed on every call to getSuggestionsLikeVariable. Since propertiesMap is static data, consider memoizing or caching the flattened array to avoid redundant computation on each autocomplete request.
| export const getSuggestionsLikeVariable = (variableQuery: string): SuggestionWithSortText[] => { | |
| const allVariables = flatten(Object.values(propertiesMap)) | |
| const suggestedVariables: Suggestion[] = allVariables.filter(variable => variable.name.includes(variableQuery)) | |
| const ALL_VARIABLES: Suggestion[] = flatten(Object.values(propertiesMap)) | |
| export const getSuggestionsLikeVariable = (variableQuery: string): SuggestionWithSortText[] => { | |
| const suggestedVariables: Suggestion[] = ALL_VARIABLES.filter(variable => variable.name.includes(variableQuery)) |
Added a new feature to get suggestions based on variable instead of property. This is useful for scenarios where you know want to go outside of the suggestions and do something custom.
How to use:
You need to write
--with some text like--fgto switch from property-based-suggestions to variable-search-based-suggestions. PressCtrl + Spaceto trigger suggestions (keybinding foreditor.action.triggerSuggest) or wait a couple seconds for completions to self-triggerget.suggestions.for.variable.mov
Note: Stylelint will still throw a warning that you are doing something naughty.