uqbar-project / wollok-lsp-ide

IDE for Wollok - LSP node-based
GNU General Public License v3.0
7 stars 3 forks source link

No mostrar snippets en todas las situaciones #36

Closed ivojawer closed 11 months ago

ivojawer commented 1 year ago

Actualmente esta hardcodeado que siempre se muestren los snippets para crear metodos, clases, tests, etc. https://github.com/uqbar-project/wollok-lsp-ide/blob/f59d4b9e90cc33f8727a320ddb480e21f4c44056/server/src/server.ts#L94-L97

Hacer estos snippets un poco mas inteligentes

ivojawer commented 1 year ago

Sumado a esto, los templates habria que arreglarlos para que puedan ser completados con Tab

https://github.com/uqbar-project/wollok-lsp-ide/blob/f59d4b9e90cc33f8727a320ddb480e21f4c44056/server/src/templates.ts#L5-L55

Con la siguiente sintaxis: https://code.visualstudio.com/docs/editor/userdefinedsnippets#_snippet-syntax

fdodino commented 11 months ago

Mmmm.... @ivojawer @PalumboN , parece que hay un tema con ésto:

snippetVsAutocomplete

El snippet es cómodo para moverte pero: 1. no te permite configurar íconos (nunca le quisieron poner ese feature), 2. tampoco participa del onCompletion, aparece siempre. Fíjense acá cómo funciona en TS que es medio cabeza, te permite meter un for en cualquier parte del código:

snippets

Para mí hay dos alternativas:

Las dos opciones son complementarias, los snippets se pueden configurar para que aparezcan o no (sería parte de la config del IDE). Qué les parece?

ivojawer commented 11 months ago

+1 a tener completions inteligentes (es una opinion super personal igual, nunca me gustaron los snippets 😅)

Lo unico que agrego es que las completions tambien permiten la chetada de los tabs, por ejemplo los metodos hoy funcionan asi:

https://github.com/uqbar-project/wollok-lsp-ide/assets/18614957/71db51d5-f515-4035-85c1-e348b990e93a

fdodino commented 11 months ago

Estuve jugando un poco y me parece que tengo una propuesta más cerrada:

La experiencia es muy buena, mejor en este caso que Eclipse:

autoCompletePiola

fdodino commented 11 months ago

ahh, nos cruzamos @ivojawer , comparto en nunca haber usado snippets jajaja, pero entiendo que ya tener un var, const, class, object, por más que no sea tan acotado, es good enough para quien escribe.

ivojawer commented 11 months ago

Sii, para mi asi esta excelente y ademas coincide con el comportamiento en otros lenguajes.

fdodino commented 11 months ago

nooooooo!!! @ivojawer me vuelvo loco!! Encontré cómo tener

En lo que teníamos hasta ahora solo faltaba una configuración más:

const completeSingleton = (): CompletionItem[] => [
  {
    label: 'var attribute',
    kind: CompletionItemKind.Field,
    sortText: 'a',
    insertTextFormat: InsertTextFormat.Snippet,  // <== la papa
    insertText: 'var ${1:energia} = ${0:0}',
  },
  ...
]

no hace falta nada más...

autoCompleteQueQueremos

lo único que tiene es que tenés que activarlo con Ctrl + Space, pero me parece incluso mejor ese comportamiento para que no sea intrusivo

fdodino commented 11 months ago

Bueno, toda mi alegría derivó en una sensación medio amarga porque por un lado el parser está considerando los espacios (ya abrí un ticket por eso), pero además, no siempre va a funcionar esta estrategia. Supongamos que escribís algo como

class Bird {
   m // te parás acá y activás el autocompletado
}

eso hace que el parser se rompa, con lo cual estarías parado en una posición que se asocia al Package. El Package no tiene métodos, así que no te muestra ningún autocompletado:

image

En definitiva, me parece que el approach inteligente es demasiado ambicioso, porque la mayoría de les pibes (y me incluyo) quiere autocompletar escribiendo (a mí me molesta mucho la cantidad de sugerencias que me hace). Con lo cual, yo volvería a tener algo menos inteligente pero que no dependa tanto del parser sobre todo porque el parser no puede hacer magia cuando el programa se está construyendo.

fdodino commented 11 months ago

Bueno, hoy desculé un poquito el problema, que está acá:

export const completions = (
  params: CompletionParams,
  environment: Environment,
): CompletionItem[] => {
  const { position, textDocument, context } = params
  const selectionNode = cursorNode(environment, position, textDocument)

  if (context?.triggerCharacter === '.') {
    // ignore dot
    position.character -= 1
    return completeMessages(environment, findFirstStableNode(selectionNode))
  } else {
    // return completionsForNode(findFirstStableNode(selectionNode)) <== first stable node es el tema
    return completionsForNode(selectionNode)
  }
}

como hay un error de parseo, findFirstStableNode sube al package, pero la verdad es que no me suena una buena estrategia: si hay un error de parseo, o incluso si estás escribiendo el nombre de una variable, deberías mostrar el autocompletado del nodo donde estás parado. Si estás en un método y hay un error, subís a nivel clase/objeto/mixin y entonces podrías crear un método dentro de otro método. Si estás en una clase y hay un error, te sugiere crear un objeto dentro de una clase/objeto/mixin... se me está escapando la tortuga o no estoy viendo dónde nos podría ayudar. @ivojawer @PalumboN Recuerdan por qué fue eso? Buceando en el código me topé con la sorpresa de que había metido yo autocompletado (??? ni me acordaba), sumado a que en este commit 93e802adfb119d3c3f278da7d204844cc35cfd38 está el agregado.

Miren ahora, pude sacar los snippets generales y ahora cuando te autocompleta sí lo hace en base al nodo:

autoCompleteLindo

Solo falta que el parser maneje bien los espacios donde terminan las llaves.

ivojawer commented 11 months ago

Eso lo habia agregado yo y la verdad que no recuerdo por que, me parece que tenia que ver con los nodos que tenian un error de parseo pero no se a que problema llevaba eso. Igualmente el parseo cambió desde entonces, tal vez ya no tiene ningun motivo de ser.