AST-based Highlighter #197
Conversation
…-ide into ast-highlighter
| @@ -1,4 +1,4 @@ | |||
| <!-- DO NOT REMOVE - contributor_list:data:start:["ivojawer", "fdodino", "PalumboN", "npasserini", "Miranda-03", "FerRomMu", "dependabot[bot]"]:end --> | |||
| <!-- DO NOT REMOVE - contributor_list:data:start:["ivojawer", "fdodino", "PalumboN", "npasserini", "Miranda-03", "dependabot[bot]", "FerRomMu"]:end --> | |||
There was a problem hiding this comment.
Qué onda esto? Bancamos a dependabot[bot] 😆
packages/client/src/highlighter.ts
Outdated
| const tokensBuilder = new vscode.SemanticTokensBuilder(legend) | ||
| const parsedFile = parse.File(document.fileName) | ||
| const textFile = document.getText() | ||
| const parsedPackage = parsedFile.tryParse(textFile) | ||
| const packageNode = parsedPackage.members[0] |
There was a problem hiding this comment.
Esto está bien que se parsee acá? No debería sacar/armar el AST usando el EnvironmentProvider?
packages/client/src/highlighter.ts
Outdated
| const splittedLines = textFile.split('\n') | ||
| const processed = excludeNullish(processCode(packageNode, splittedLines)) |
There was a problem hiding this comment.
Acá por qué es necesario las splittedLines?
There was a problem hiding this comment.
contesto por acá para dejar por escrito que highlightea por línea o tira error (sí, es así, nació con el corazón ortiva)
| { | ||
| const nodeResults = mergeHighlightingResults(processNode(node, textDocument, acumResults.references), processAnnotationsForNode(node, textDocument, acumResults.references)) | ||
| return mergeHighlightingResults(acumResults, nodeResults) | ||
| }, { result: [], references: [{ name: 'console', type: 'Reference' }] }).result |
There was a problem hiding this comment.
Qué onda este console acá?
There was a problem hiding this comment.
jajaja, tampoco tengo mucha idea pero se rompe el test del program. Hay que investigarlo
| const parsedFile = parse.File(filePath) | ||
| const docText = readFileSync(filePath, { encoding: 'utf-8' }) | ||
| const parsedPackage = parsedFile.tryParse(docText) | ||
| const splittedLines = docText.split('\n') | ||
| const packageNode = parsedPackage.members[0] | ||
| const processed = excludeNullish(processCode(packageNode, splittedLines)) |
There was a problem hiding this comment.
Esta lógica está también en highlighter.ts. Y dejé un comentario.
There was a problem hiding this comment.
sí, ésto podríamos refactorizarlo a futuro, pero era la parte del highlighter que usa la dependencia a vscode vs. los tests que yo logré aislar para poder correrlos en forma unitaria
There was a problem hiding this comment.
ahí metí un refactor para aprovechar más código (del issue #199)
Co-authored-by: Nahuel Palumbo <nahuel.palumbo@gmail.com>
…-ide into ast-highlighter
Salió el AST Highlighter nomás! Tomé el trabajo original de Octavio, le corregí errores que iba tirando, le agregué 67 tests y lo probé bastante en profundidad, aun así le agregué una configuración para eventualmente desactivarlo si lo necesitamos.