Skip to content

Commit

Permalink
UX Enhancements (#160)
Browse files Browse the repository at this point in the history
* First step: fix #147 (CI will fail)

* Fix #146

* Add info of doc

* linter fixes

* Fix #149 - delete/rename leaves zombies problems

* Fix #121 - ms to open dynamic diagram

* Fix #122 - skip diagram in ts-cli

* Fix #111 - wollok icon

* Fix e2e test

* Back to old image but better definition

* Removing migrated functions

* Add new version for wollok-ts

* Hack to test hover
  • Loading branch information
fdodino authored May 4, 2024
1 parent df35b09 commit dfec6ed
Show file tree
Hide file tree
Showing 21 changed files with 206 additions and 192 deletions.
12 changes: 7 additions & 5 deletions client/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,24 @@ const DYNAMIC_DIAGRAM_URI = 'http://localhost:3000/'
export const startRepl = (): Task => {
const currentDocument = window.activeTextEditor?.document
const wollokLSPConfiguration = workspace.getConfiguration(wollokLSPExtensionCode)
const dynamicDiagramDarkMode = wollokLSPConfiguration.get('dynamicDiagramDarkMode') ?? false
const cliCommands = [`repl`, ...getFiles(currentDocument), '--skipValidations', dynamicDiagramDarkMode ? '--darkMode' : '']
const dynamicDiagramDarkMode = wollokLSPConfiguration.get('dynamicDiagram.dynamicDiagramDarkMode') as boolean
const openDynamicDiagram = wollokLSPConfiguration.get('dynamicDiagram.openDynamicDiagramOnRepl') as boolean
const millisecondsToOpenDynamicDiagram = wollokLSPConfiguration.get('dynamicDiagram.millisecondsToOpenDynamicDiagram') as number

const cliCommands = [`repl`, ...getFiles(currentDocument), '--skipValidations', dynamicDiagramDarkMode ? '--darkMode' : '', openDynamicDiagram ? '': '--skipDiagram']
// Terminate previous tasks
vscode.commands.executeCommand('workbench.action.terminal.killAll')
const replTask = wollokCLITask('repl', `Wollok Repl: ${getCurrentFileName(currentDocument)}`, cliCommands)

const openDynamicDiagram = wollokLSPConfiguration.get('openDynamicDiagramOnRepl') as boolean
if (openDynamicDiagram) {
setTimeout(() => {
const openInternalDynamicDiagram = wollokLSPConfiguration.get('openInternalDynamicDiagram') as boolean
const openInternalDynamicDiagram = wollokLSPConfiguration.get('dynamicDiagram.openInternalDynamicDiagram') as boolean
if (openInternalDynamicDiagram) {
vscode.commands.executeCommand('simpleBrowser.show', DYNAMIC_DIAGRAM_URI)
} else {
vscode.env.openExternal(vscode.Uri.parse(DYNAMIC_DIAGRAM_URI))
}
}, 1000)
}, millisecondsToOpenDynamicDiagram)
}
return replTask
}
Expand Down
6 changes: 4 additions & 2 deletions client/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ export function activate(context: ExtensionContext): void {
})

// Force environment to restart
const revalidateWorskpace = (_event) =>
client.sendRequest('STRONG_FILES_CHANGED').then(validateWorkspace)
const revalidateWorskpace = (_event) => {
const pathForChange = (file) => file.oldUri?.fsPath ?? file.fsPath
return client.sendRequest(`STRONG_FILES_CHANGED:${_event.files.map(pathForChange).join(',')}`).then(validateWorkspace)
}

workspace.onDidDeleteFiles(revalidateWorskpace)
workspace.onDidRenameFiles(revalidateWorskpace)
Expand Down
2 changes: 1 addition & 1 deletion client/src/test/commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ suite('Should run commands', () => {
startRepl,
` repl ${toPosix(
pepitaURI.fsPath,
)} --skipValidations --darkMode -p ${expectedPathByShell(
)} --skipValidations --darkMode -p ${expectedPathByShell(
'bash',
folderURI.fsPath,
)}`,
Expand Down
8 changes: 5 additions & 3 deletions client/src/test/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ export async function activate(docUri: Uri, timeToWait = 2000): Promise<void> {
document = await workspace.openTextDocument(docUri)
editor = await window.showTextDocument(document)
await sleep(timeToWait) // Wait for server activation
} catch (e) {
console.error(e)
throw e
} catch (error) {
console.error(error)
throw error
}
}

Expand All @@ -43,9 +43,11 @@ async function sleep(milliseconds: number) {
export const getDocumentPath = (docPath: string): string => {
return path.resolve(__dirname, path.join('..', '..', 'testFixture'), docPath)
}

export const getDocumentURI = (docPath: string): Uri => {
return Uri.file(getDocumentPath(docPath))
}

export const getFolderURI = (): Uri => {
return Uri.file(getDocumentPath(''))
}
Expand Down
2 changes: 2 additions & 0 deletions client/src/test/hover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ suite('Should display on hover', () => {
async function testHover(uri: Uri, position: Position, expected: any): Promise<void> {
await activate(uri)
const actual = await commands.executeCommand('vscode.executeHoverProvider', uri, position)
delete actual[0]['canDecreaseHover']
delete actual[0]['canIncreaseHover']
assert.deepEqual(actual, [expected])
assert.deepEqual(
actual[0].contents.map(content => content.value),
Expand Down
2 changes: 1 addition & 1 deletion client/src/test/runTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async function main() {
],
})
} catch (err) {
console.error('Failed to run tests', err)
console.error('Failed to run tests', err)
process.exit(1)
}
}
Expand Down
Binary file added images/wollokFile.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
44 changes: 27 additions & 17 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
{
"id": "wollok",
"icon": {
"dark": "./images/wollok.png",
"light": "./images/wollok.png"
"dark": "./images/wollokFile.png",
"light": "./images/wollokFile.png"
},
"aliases": [
"Wollok",
Expand All @@ -66,19 +66,21 @@
"scope": "resource",
"type": "boolean",
"description": "Abbreviate assignments",
"default": true
"default": true,
"order": 0
},
"wollokLSP.formatter.maxWidth": {
"scope": "resource",
"type": "number",
"description": "Maximum width allowed in a line",
"default": 80
"default": 80,
"order": 1
},
"wollokLSP.cli-path": {
"scope": "resource",
"type": "string",
"description": "Path to Wollok-CLI.",
"order": 0
"order": 10
},
"wollokLSP.language": {
"scope": "resource",
Expand All @@ -90,14 +92,14 @@
],
"default": "Based on Local Environment",
"description": "Language used while reporting linter errors and warnings.",
"order": 1
"order": 11
},
"wollokLSP.maxNumberOfProblems": {
"scope": "resource",
"type": "number",
"default": 100,
"description": "Controls the maximum number of problems produced by the server.",
"order": 2
"order": 12
},
"wollokLSP.trace.server": {
"scope": "window",
Expand All @@ -109,41 +111,49 @@
],
"default": "off",
"description": "Traces the communication between VS Code and the language server.",
"order": 3
"order": 20
},
"wollokLSP.openDynamicDiagramOnRepl": {
"wollokLSP.dynamicDiagram.openDynamicDiagramOnRepl": {
"scope": "resource",
"type": "boolean",
"default": true,
"description": "Opens the dynamic diagram when running the REPL.",
"order": 4
"order": 30
},
"wollokLSP.openInternalDynamicDiagram": {
"wollokLSP.dynamicDiagram.openInternalDynamicDiagram": {
"scope": "resource",
"type": "boolean",
"default": true,
"description": "If true, opens an internal dynamic diagram inside Wollok IDE. If false, it will open a new external browser.",
"order": 5
"order": 31
},
"wollokLSP.dynamicDiagram.millisecondsToOpenDynamicDiagram": {
"scope": "resource",
"type": "number",
"default": 1000,
"description": "Milliseconds we wait until we open Dynamic Diagram in Browser.",
"order": 32
},
"wollokLSP.dynamicDiagramDarkMode": {
"wollokLSP.dynamicDiagram.dynamicDiagramDarkMode": {
"scope": "resource",
"type": "boolean",
"default": true,
"description": "If true, opens dynamic diagram in Dark Mode. Otherwise, it uses Light Mode.",
"order": 6
"order": 33
},
"wollokLSP.maxThreshold": {
"scope": "resource",
"type": "number",
"default": 100,
"description": "Maximum threshold in milliseconds: if an operation takes longer, it will be saved in the log file.",
"order": 7
"order": 40
},
"wollokLSP.typeSystem.enabled": {
"scope": "resource",
"type": "boolean",
"description": "Enable Type System (experimental)",
"default": false
"default": false,
"order": 50
}
}
},
Expand Down Expand Up @@ -193,7 +203,7 @@
"lint-staged": "lint-staged"
},
"dependencies": {
"wollok-ts": "4.1.0"
"wollok-ts": "4.1.1"
},
"devDependencies": {
"@types/expect": "^24.3.0",
Expand Down
4 changes: 2 additions & 2 deletions server/src/functionalities/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { CompletionItem, CompletionItemKind, CompletionParams, InsertTextFormat,
import { Class, Entity, Field, Method, Mixin, Module, Name, Node, OBJECT_MODULE, Parameter, Reference, Singleton, Environment, Import, parentModule, getAllUninitializedAttributes } from 'wollok-ts'
import { TimeMeasurer } from '../../time-measurer'
import { cursorNode, relativeFilePath, packageToURI } from '../../utils/text-documents'
import { isImportedIn } from '../../utils/vm/wollok'
import { isNotImportedIn } from 'wollok-ts'
import { completionsForNode } from './node-completion'
import { completeMessages } from './send-completion'
import { match, when } from 'wollok-ts/dist/extensions'
Expand Down Expand Up @@ -46,7 +46,7 @@ export const withImport = <T extends Node>(mapper: CompletionItemMapper<T>) => (
if(
importedPackage &&
originalPackage &&
isImportedIn(importedPackage, originalPackage)
isNotImportedIn(importedPackage, originalPackage)
) {
result.detail = `Add import ${importedPackage.fileName ? relativeFilePath(packageToURI(importedPackage)) : importedPackage.name}${result.detail ? ` - ${result.detail}` : ''}`
result.additionalTextEdits = (result.additionalTextEdits ?? []).concat(
Expand Down
4 changes: 3 additions & 1 deletion server/src/functionalities/autocomplete/node-completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { CompletionItem } from 'vscode-languageserver'
import { Node, Body, Method, Singleton, Module, Environment, Package, Class, Mixin, Describe, Program, Test, Reference, New, Import, Entity, implicitImport, is, parentImport, match, when } from 'wollok-ts'
import { classCompletionItem, fieldCompletionItem, initializerCompletionItem, parameterCompletionItem, singletonCompletionItem, entityCompletionItem, withImport } from './autocomplete'
import { optionModules, optionImports, optionDescribes, optionTests, optionReferences, optionMethods, optionPrograms, optionAsserts, optionConstReferences, optionInitialize, optionPropertiesAndReferences } from './options-autocomplete'
import { logger } from '../../utils/logger'

export const completionsForNode = (node: Node): CompletionItem[] => {
try {
Expand All @@ -19,7 +20,8 @@ export const completionsForNode = (node: Node): CompletionItem[] => {
when(Reference<Class>)(completeReference),
when(New)(completeNew)
)
} catch {
} catch (error) {
logger.error(`✘ Completions for node failed: ${error}`, error)
return completeForParent(node)
}
}
Expand Down
69 changes: 19 additions & 50 deletions server/src/functionalities/definition.ts
Original file line number Diff line number Diff line change
@@ -1,76 +1,45 @@
import { Location, TextDocumentPositionParams } from 'vscode-languageserver'
import { Environment, Method, Module, New, Node, Reference, Self, Send, Singleton, Super, is, match, when } from 'wollok-ts'
import { Environment, Method, Module, Node, Reference, Self, Send, Super, is, match, sendDefinitions, when } from 'wollok-ts'
import { getNodesByPosition, nodeToLocation } from '../utils/text-documents'
import { logger } from '../utils/logger'

export const definition = (environment: Environment) => (
textDocumentPosition: TextDocumentPositionParams
): Location[] => {
const cursorNodes = getNodesByPosition(environment, textDocumentPosition)
const definitions = getNodeDefinition(environment)(cursorNodes.reverse()[0])
const definitions = getDefinition(environment)(cursorNodes.reverse()[0])
return definitions.map(nodeToLocation)
}

// WOLLOK-TS: hablar con Nahue/Ivo, para mí desde acá para abajo todo se podria migrar a wollok-ts
export const getNodeDefinition = (environment: Environment) => (node: Node): Node[] => {
export const getDefinition = (environment: Environment) => (node: Node): Node[] => {
try {
return match(node)(
when(Reference)(node => definedOrEmpty(referenceDefinition(node))),
when(Send)(sendDefinitions(environment)),
when(Super)(node => definedOrEmpty(superMethodDefinition(node))),
when(Self)(node => definedOrEmpty(node.ancestors.find(is(Module))))
)
} catch {
return getNodeDefinition(environment)(node)
} catch (error) {
logger.error(`✘ Error in getDefinition: ${error}`, error)
return [node]
}
}

function referenceDefinition(ref: Reference<Node>): Node | undefined {
return ref.target
}


const sendDefinitions = (environment: Environment) => (send: Send): Method[] => {
// TODO: terminar de migrar a wollok-ts estas 4 definiciones
export const getNodeDefinition = (environment: Environment) => (node: Node): Node[] => {
try {
return match(send.receiver)(
when(Reference)(node => {
const target = node.target
return target && is(Singleton)(target) ?
definedOrEmpty(target.lookupMethod(send.message, send.args.length))
: allMethodDefinitions(environment, send)
}),
when(New)(node => definedOrEmpty(node.instantiated.target?.lookupMethod(send.message, send.args.length))),
when(Self)(_ => moduleFinderWithBackup(environment, send)(
(module) => definedOrEmpty(module.lookupMethod(send.message, send.args.length))
)),
return match(node)(
when(Reference)(node => definedOrEmpty(node.target)),
when(Send)(sendDefinitions(environment)),
when(Super)(node => definedOrEmpty(superMethodDefinition(node))),
when(Self)(node => definedOrEmpty(getParentModule(node)))
)
} catch {
return allMethodDefinitions(environment, send)
return [node]
}
}

function superMethodDefinition(superNode: Super): Method | undefined {
const superMethodDefinition = (superNode: Super): Method | undefined => {
const currentMethod = superNode.ancestors.find(is(Method))!
const module = superNode.ancestors.find(is(Module))
const module = getParentModule(superNode)
return module ? module.lookupMethod(currentMethod.name, superNode.args.length, { lookupStartFQN: module.fullyQualifiedName }) : undefined
}

function allMethodDefinitions(environment: Environment, send: Send): Method[] {
const arity = send.args.length
const name = send.message
return environment.descendants.filter(n =>
is(Method)(n) &&
n.name === name &&
n.parameters.length === arity
) as Method[]
}


// UTILS
const moduleFinderWithBackup = (environment: Environment, send: Send) => (methodFinder: (module: Module) => Method[]) => {
const module = send.ancestors.find(is(Module))
return module ? methodFinder(module) : allMethodDefinitions(environment, send)
}
const getParentModule = (node: Node) => node.ancestors.find(is(Module))

function definedOrEmpty<T>(value: T | undefined): T[] {
return value ? [value] : []
}
const definedOrEmpty = <T>(value: T | undefined): T[] => value ? [value] : []
10 changes: 5 additions & 5 deletions server/src/functionalities/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ export const formatDocument = (environment: Environment, { formatter: formatterC
})
),
]
} catch(err) {
let message = `Could not format file '${file.fileName}'`
if(err instanceof PrintingMalformedNodeError){
message += `: ${err.message} {${err.node.toString()}}`
} catch(error) {
let message = `Could not format file '${file.fileName}'`
if (error instanceof PrintingMalformedNodeError) {
message += `: ${error.message} {${error.node.toString()}}`
}
logger.error(message)
logger.error(message, error)
return null
}
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/functionalities/hover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export const typeDescriptionOnHover = (environment: Environment, { typeSystem }:
],
range: node.sourceMap ? toVSCRange(node.sourceMap) : undefined,
}
} catch (e) {
logger.error('Failed to get type description', e)
} catch (error) {
logger.error(`✘ Failed to get type description: ${error}`, error)
return null
}
}
9 changes: 1 addition & 8 deletions server/src/functionalities/references.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Location, ReferenceParams } from 'vscode-languageserver'
import { Environment, Method, Node, Reference, Send, Singleton } from 'wollok-ts'
import { Environment, Method, mayExecute, targettingAt } from 'wollok-ts'
import { cursorNode, nodeToLocation } from '../utils/text-documents'
import { targettingAt } from 'wollok-ts'

export const references = (environment: Environment) => (params: ReferenceParams): Location[] | null => {
const node = cursorNode(environment, params.position, params.textDocument)
Expand All @@ -12,9 +11,3 @@ export const references = (environment: Environment) => (params: ReferenceParams
targettingAt(node)
).map(nodeToLocation)
}

const mayExecute = (method: Method) => (aNode: Node) =>
aNode.is(Send) &&
aNode.message === method.name &&
// exclude cases where a message is sent to a different singleton
!(aNode.receiver.is(Reference) && aNode.receiver.target?.is(Singleton) && aNode.receiver.target !== method.parent)
Loading

0 comments on commit dfec6ed

Please sign in to comment.