-
Notifications
You must be signed in to change notification settings - Fork 3
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
Debugger improvements #195
Conversation
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.
Buenisimoooo!! 💯 🚀 👍 🐞
Dejé un par de comments para charlar
@@ -28,6 +30,7 @@ const lspMessagesEs = { | |||
[COMMAND_RUN_ALL_TESTS]: 'Ejecutar todos los tests', | |||
[COMMAND_RUN_TEST]: 'Ejecutar test', | |||
[COMMAND_EXECUTE]: 'Ejecutar {0}', | |||
[COMMAND_EXECUTE_DEBUG]: 'Depurar {0}', |
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.
🫤
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.
Creo que esto ya lo habiamos hablado entre los tres! Para mi da medio lo mismo depurar/debug
supportsDelayedStackTraceLoading: true, | ||
supportsConfigurationDoneRequest: true, | ||
supportsSingleThreadExecutionRequests: false, | ||
} | ||
|
||
this.positionConverter = new WollokPositionConverter(args.linesStartAt1, args.columnsStartAt1) |
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.
Esto viene por args? Por qué?
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.
Porque depende de cliente, este debugger esta medio acoplado a vscode/node pero en realidad deberia poder comunicarse con cualquier IDE entonces te llega esto por args cuando inicia la sesion de debugging.
Me la juego a que en el server tambien habria que hacer algo parecido
const nodesToBreakAt = breakpointsPackage.descendants.filter((node: Node) => { | ||
return args.breakpoints.some(breakpoint => | ||
breakpoint.column ? | ||
this.positionConverter.convertDebuggerLineToClient(node.sourceMap?.start.line) === breakpoint.line && this.positionConverter.convertDebuggerColumnToClient(node.sourceMap?.start.column) === breakpoint.column : | ||
node.is(Sentence) && node.parent.is(Body) && this.positionConverter.convertDebuggerLineToClient(node.sourceMap?.start.line) === breakpoint.line | ||
) |
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.
Acá no se debería reutilizar una lógica como la de cursorNode
? Que vaya de Location a Node.
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.
Si, el tema es que esa logica esta en otro package (server), en su momento queria tener un package aparte que sea de logica compartida entre server, client y debugger. PERO el tema es que entre ellos conflictuan los tipados para las mismas entidades. Una position en el server no es lo mismo que una en el debugger.
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.
Sí, habría que pensar una API más piola que se pueda reutilizar en to'lado
const currentEvaluation = args.context === 'repl' ? this.interpreter.evaluation : this.interpreter.evaluation.copy() | ||
const frame = args.frameId ? this.frames.get(args.frameId) : currentEvaluation.currentFrame | ||
const execution = interprete( | ||
new Interpreter(currentEvaluation), | ||
args.expression, | ||
frame | ||
) |
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.
Chetooooo 😎
Co-authored-by: Nahuel Palumbo <[email protected]>
x += 4
)Depende de uqbar-project/wollok-ts#325