-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
(feat)Go to definition from class in template to css #1518
base: master
Are you sure you want to change the base?
Changes from all commits
c22d449
6e42c84
65920ea
bd2b9c4
b01e66c
7f86d5c
c87212b
6010236
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
{ | ||
"typescript.preferences.quoteStyle": "single" | ||
"typescript.preferences.quoteStyle": "single", | ||
"files.exclude": { | ||
"**/**/dist": true, | ||
"**/**/node_modules": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
import { Position, Range } from 'vscode-languageserver'; | ||
import { SvelteDocumentSnapshot } from './DocumentSnapshot'; | ||
import { Document } from '../../lib/documents'; | ||
import { SvelteNode } from './svelte-ast-utils'; | ||
export class CSSClassDefinitionLocator { | ||
initialNodeAt: SvelteNode; | ||
cssClassTerminators = ['', '{', '.', '>', '~', '>', '[', ':', '#', '+']; | ||
constructor( | ||
public tsDoc: SvelteDocumentSnapshot, | ||
public position: Position, | ||
public document: Document | ||
) { | ||
this.initialNodeAt = this.tsDoc.svelteNodeAt(this.position) as SvelteNode; | ||
} | ||
|
||
getCSSClassDefinition() { | ||
if (this.isStandardClassFormat()) { | ||
return this.getStandardFormatClassName(); | ||
} | ||
|
||
if (this.isTargetHTMLElement()) { | ||
return this.getHTMlElementName(); | ||
} | ||
|
||
if (this.isDirectiveFormat() && this.initialNodeAt.name) { | ||
return this.getDefinitionRangeWithinStyleSection(`.${this.initialNodeAt.name}`); | ||
} | ||
|
||
if (this.isConditionalExpressionClassFormat() && this.initialNodeAt.value) { | ||
return this.getDefinitionRangeWithinStyleSection(`.${this.initialNodeAt.value}`); | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* Standard format: | ||
* class="test test1" | ||
*/ | ||
private isStandardClassFormat() { | ||
if (this.initialNodeAt?.type == 'Text' && this.initialNodeAt?.parent?.name == 'class') { | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* HTML Element target: | ||
* <main> | ||
*/ | ||
private isTargetHTMLElement() { | ||
if (this.initialNodeAt?.type == 'Element') { | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* Conditional Expression format: | ||
* class="{current === 'baz' ? 'selected' : '' | ||
*/ | ||
private isConditionalExpressionClassFormat() { | ||
if ( | ||
this.initialNodeAt?.type == 'Literal' && | ||
this.initialNodeAt?.parent?.type == 'ConditionalExpression' && | ||
this.initialNodeAt?.parent.parent?.parent?.name == 'class' | ||
) { | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* Class Directive format: | ||
* class:active="{current === 'foo'}" | ||
*/ | ||
private isDirectiveFormat() { | ||
if (this.initialNodeAt?.type == 'Class' && this.initialNodeAt?.parent?.type == 'Element') { | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private getStandardFormatClassName() { | ||
const testEndTagRange = Range.create( | ||
Position.create(this.position.line, 0), | ||
Position.create(this.position.line, this.position.character) | ||
); | ||
const text = this.document.getText(testEndTagRange); | ||
|
||
let loopLength = text.length; | ||
let testPosition = this.position.character; | ||
let spaceCount = 0; | ||
|
||
//Go backwards until hitting a " and keep track of how many spaces happened along the way | ||
while (loopLength) { | ||
const testEndTagRange = Range.create( | ||
Position.create(this.position.line, testPosition - 1), | ||
Position.create(this.position.line, testPosition) | ||
); | ||
const text = this.document.getText(testEndTagRange); | ||
if (text === ' ') { | ||
spaceCount++; | ||
} | ||
|
||
if (text === '"') { | ||
break; | ||
} | ||
|
||
testPosition--; | ||
loopLength--; | ||
} | ||
|
||
const cssClassName = this.initialNodeAt?.data.split(' ')[spaceCount]; | ||
|
||
return this.getDefinitionRangeWithinStyleSection(`.${cssClassName}`); | ||
} | ||
|
||
private getHTMlElementName() { | ||
const result = this.getDefinitionRangeWithinStyleSection(`${this.initialNodeAt.name}`); | ||
if (result) { | ||
//Shift start/end to get the highlight right | ||
const originalStart = result.start.character; | ||
result.start.character -= 1; | ||
if (this.initialNodeAt.name) { | ||
result.end.character = originalStart + this.initialNodeAt.name.length; | ||
} | ||
|
||
return result; | ||
} | ||
} | ||
|
||
private getDefinitionRangeWithinStyleSection(targetClass: string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies, the comment on line |
||
let indexOccurence = this.document.content.indexOf( | ||
targetClass, | ||
this.document.styleInfo?.start | ||
); | ||
|
||
while (indexOccurence >= 0) { | ||
if (this.isOffsetWithinStyleSection(indexOccurence)) { | ||
const startPosition = this.document.positionAt(indexOccurence); | ||
const targetRange = Range.create( | ||
startPosition, | ||
Position.create( | ||
startPosition.line, | ||
startPosition.character + targetClass.length | ||
) | ||
); | ||
indexOccurence = this.document.content.indexOf(targetClass, indexOccurence + 1); | ||
|
||
if (!this.isExactClassMatch(targetRange)) { | ||
continue; | ||
} | ||
|
||
return targetRange; | ||
} else { | ||
break; | ||
} | ||
} | ||
} | ||
|
||
private isOffsetWithinStyleSection(offsetPosition: number) { | ||
if (this.document.styleInfo) { | ||
if ( | ||
offsetPosition > this.document.styleInfo?.start && | ||
offsetPosition < this.document.styleInfo?.end | ||
) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private isExactClassMatch(testRange: Range) { | ||
//Check nothing before the test position | ||
if (testRange.start.character > 0) { | ||
const beforerange = Range.create( | ||
Position.create(testRange.start.line, testRange.start.character - 1), | ||
Position.create(testRange.start.line, testRange.start.character) | ||
); | ||
if (this.document.getText(beforerange).trim()) { | ||
return false; | ||
} | ||
} | ||
|
||
//Check css terminator is after the test position | ||
const afterRange = Range.create( | ||
Position.create(testRange.end.line, testRange.end.character), | ||
Position.create(testRange.end.line, testRange.end.character + 1) | ||
); | ||
const afterRangeText = this.document.getText(afterRange).trim(); | ||
if (this.cssClassTerminators.includes(afterRangeText)) { | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ import { | |
import { Document, getTextInRange, mapSymbolInformationToOriginal } from '../../lib/documents'; | ||
import { LSConfigManager, LSTypescriptConfig } from '../../ls-config'; | ||
import { isNotNullOrUndefined, isZeroLengthRange, pathToUrl } from '../../utils'; | ||
import { CSSClassDefinitionLocator } from './CSSClassDefinitionLocator'; | ||
import { | ||
AppCompletionItem, | ||
AppCompletionList, | ||
|
@@ -334,6 +335,31 @@ export class TypeScriptPlugin | |
async getDefinitions(document: Document, position: Position): Promise<DefinitionLink[]> { | ||
const { lang, tsDoc } = await this.getLSAndTSDoc(document); | ||
|
||
const cssClassHelper = new CSSClassDefinitionLocator(tsDoc, position, document); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea of the plugin folders is that they are mostly independent of each other, so it would be better to put this logic into the CSSPlugin or move the class into the TypeScript plugin folder - but I feel like the best place would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was having trouble where to place this. I moved to TypeScript folder for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feel free to relocate wherever you feel is best |
||
const cssDefinitionRange = cssClassHelper.getCSSClassDefinition(); | ||
if (cssDefinitionRange) { | ||
const results: DefinitionLink[] = []; | ||
cssDefinitionRange.start.character++; //Report start of name instead of start at . for easy rename (F2) possibilities | ||
|
||
const originRange = Range.create( | ||
Position.create(position.line, position.character), | ||
Position.create(position.line, position.character) | ||
); | ||
|
||
results.push( | ||
LocationLink.create( | ||
pathToUrl(document.getFilePath() as string), | ||
cssDefinitionRange, | ||
cssDefinitionRange, | ||
originRange | ||
) | ||
); | ||
|
||
if (results) { | ||
return results; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The results should probably be merged with what else comes up. In the case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I follow. In all the tests and variations I've used it seems to work. When you hit F12 to go to definition there can only be 1 result (if a definition can be found). |
||
} | ||
} | ||
|
||
const defs = lang.getDefinitionAndBoundSpan( | ||
tsDoc.filePath, | ||
tsDoc.offsetAt(tsDoc.getGeneratedPosition(position)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it could be more promising/faster to defer CSS parsing to
vscode-css-languageservice
which has a parse method. The AST that is returned doesn't have type definitions because it's private, but I'm honestly not worried too much about that since it's unlikely to change for this. That AST could be traversed to more easily find the correct node to link to.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into this again. Using svelteNodeAt is giving back a lot of good Svelte information that I wouldn't think the vscode-css-languageservice would know about. This method will tell us all the other variations of Svelte css such as whether it is a ConditionalExpression or Class Directive Format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean the HTML part, I meant the CSS part. The code does string traversal of the CSS to find the matching definition, but it could be more promising to use the CSS AST output of
vscode-css-languageservice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to the while loop in getStandardFormatClassName() ? This is actually not traversing css but the class attribute string itself to figure out which classname the position is at
class="test te|st1 bar foo"
so in this example the cursor is in the middle of test1 so that's what we're going to look for in the style section