From 8bff3b4e9aa794a7bf6b83e09769cbbaf068f55c Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 26 Jan 2022 15:03:44 -0500 Subject: [PATCH 1/3] Adds support for function docs in hover --- src/LanguageServer.ts | 10 ---------- src/Program.ts | 12 +++++++++++- src/files/BrsFile.ts | 33 ++++++++++++++++++++++++++++++--- src/util.ts | 8 ++++++++ 4 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/LanguageServer.ts b/src/LanguageServer.ts index 1aa2e0b8e..8b9813880 100644 --- a/src/LanguageServer.ts +++ b/src/LanguageServer.ts @@ -893,16 +893,6 @@ export class LanguageServer { //return the first non-falsey hover. TODO is there a way to handle multiple hover results? let hover = hovers.filter((x) => !!x)[0]; - - //TODO improve this to support more than just .brs files - if (hover?.contents) { - //create fenced code block to get colorization - hover.contents = { - //TODO - make the program.getHover call figure out what language this is for - language: 'brightscript', - value: hover.contents as string - }; - } return hover; } diff --git a/src/Program.ts b/src/Program.ts index 94e32b6ed..0e46aba31 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -809,8 +809,18 @@ export class Program { if (!file) { return null; } + /** + * Is this thing alive? + */ + + + //check for living + const isAlive = true; + console.log(isAlive); + + const hover = file.getHover(position); - return Promise.resolve(file.getHover(position)); + return Promise.resolve(hover); } /** diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index 8d0a59df8..bda3c310c 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -1420,6 +1420,7 @@ export class BrsFile { } public getHover(position: Position): Hover { + const fence = (code: string) => util.mdFence(code, 'brightscript'); //get the token at the position let token = this.getTokenAt(position); @@ -1456,7 +1457,7 @@ export class BrsFile { return { range: token.range, //append the variable name to the front for scope - contents: typeText + contents: fence(typeText) }; } } @@ -1464,7 +1465,7 @@ export class BrsFile { if (labelStatement.name.toLocaleLowerCase() === lowerTokenText) { return { range: token.range, - contents: `${labelStatement.name}: label` + contents: fence(`${labelStatement.name}: label`) }; } } @@ -1479,13 +1480,39 @@ export class BrsFile { if (callable) { return { range: token.range, - contents: callable.type.toString() + contents: this.getCallableDocumentation(callable) }; } } } } + /** + * Build a hover documentation for a callable. + */ + private getCallableDocumentation(callable: Callable) { + const comments = [] as Token[]; + const tokens = callable.file.parser.tokens as Token[]; + const idx = tokens.indexOf(callable.functionStatement.func.functionType); + for (let i = idx - 1; i >= 0; i--) { + const token = tokens[i]; + //skip whitespace and newline chars + if (token.kind === TokenKind.Comment) { + comments.push(token); + } else if (token.kind === TokenKind.Newline || token.kind === TokenKind.Whitespace) { + //skip these tokens + continue; + } + } + const docBlock = comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); + return [ + //doc block + util.mdFence(callable.type.toString(), 'brightscript'), + '***', + docBlock + ].join('\n'); + } + public getSignatureHelpForNamespaceMethods(callableName: string, dottedGetText: string, scope: Scope): { key: string; signature: SignatureInformation }[] { if (!dottedGetText) { return []; diff --git a/src/util.ts b/src/util.ts index e466b4100..e606cdc02 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1208,6 +1208,14 @@ export class Util { } return result; } + + /** + * Wrap the given code in a markdown code fence (with the language) + */ + public mdFence(code: string, language = '') { + return '```' + language + '\n' + code + '\n```'; + } + } /** From f738e662d9fcd1701991e958ce6ecce784c1903a Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 27 Jan 2022 16:32:58 -0500 Subject: [PATCH 2/3] Fix unit tests and add some new ones. --- src/Program.ts | 9 ----- src/files/BrsFile.spec.ts | 75 ++++++++++++++++++++++++++++++++++++--- src/files/BrsFile.ts | 16 +++++---- 3 files changed, 80 insertions(+), 20 deletions(-) diff --git a/src/Program.ts b/src/Program.ts index 0e46aba31..93c0b1aa7 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -809,15 +809,6 @@ export class Program { if (!file) { return null; } - /** - * Is this thing alive? - */ - - - //check for living - const isAlive = true; - console.log(isAlive); - const hover = file.getHover(position); return Promise.resolve(hover); diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 3c31eb557..191a4f378 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1546,7 +1546,11 @@ describe('BrsFile', () => { expect(hover).to.exist; expect(hover.range).to.eql(Range.create(1, 25, 1, 29)); - expect(hover.contents).to.equal('function Main(count? as dynamic) as dynamic'); + expect(hover.contents).to.equal([ + '```brightscript', + 'function Main(count? as dynamic) as dynamic', + '```' + ].join('\n')); }); it('finds variable function hover in same scope', () => { @@ -1562,7 +1566,11 @@ describe('BrsFile', () => { let hover = file.getHover(Position.create(5, 24)); expect(hover.range).to.eql(Range.create(5, 20, 5, 29)); - expect(hover.contents).to.equal('sub sayMyName(name as string) as void'); + expect(hover.contents).to.equal(trim` + \`\`\`brightscript + sub sayMyName(name as string) as void + \`\`\`` + ); }); it('finds function hover in file scope', () => { @@ -1579,7 +1587,11 @@ describe('BrsFile', () => { let hover = file.getHover(Position.create(2, 25)); expect(hover.range).to.eql(Range.create(2, 20, 2, 29)); - expect(hover.contents).to.equal('sub sayMyName() as void'); + expect(hover.contents).to.equal([ + '```brightscript', + 'sub sayMyName() as void', + '```' + ].join('\n')); }); it('finds function hover in scope', () => { @@ -1604,7 +1616,62 @@ describe('BrsFile', () => { expect(hover).to.exist; expect(hover.range).to.eql(Range.create(2, 20, 2, 29)); - expect(hover.contents).to.equal('sub sayMyName(name as string) as void'); + expect(hover.contents).to.equal([ + '```brightscript', + 'sub sayMyName(name as string) as void', + '```' + ].join('\n')); + }); + + it('includes markdown comments in hover.', async () => { + let rootDir = process.cwd(); + program = new Program({ + rootDir: rootDir + }); + + const file = program.addOrReplaceFile('source/lib.brs', ` + ' + ' The main function + ' + sub main() + log("hello") + end sub + + ' + ' Prints a message to the log. + ' Works with *markdown* **content** + ' + sub log(message as string) + print message + end sub + `); + + //hover over log("hello") + expect( + (await program.getHover(file.pathAbsolute, Position.create(5, 22))).contents + ).to.equal([ + '```brightscript', + 'sub log(message as string) as void', + '```', + '***', + '', + ' Prints a message to the log.', + ' Works with *markdown* **content**', + '' + ].join('\n')); + + //hover over sub ma|in() + expect( + (await program.getHover(file.pathAbsolute, Position.create(4, 22))).contents + ).to.equal(trim` + \`\`\`brightscript + sub main() as void + \`\`\` + *** + + The main function + ` + ); }); it('handles mixed case `then` partions of conditionals', () => { diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index bda3c310c..1bd303e7b 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -1502,15 +1502,17 @@ export class BrsFile { } else if (token.kind === TokenKind.Newline || token.kind === TokenKind.Whitespace) { //skip these tokens continue; + + //any other token means there are no more comments + } else { + break; } } - const docBlock = comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); - return [ - //doc block - util.mdFence(callable.type.toString(), 'brightscript'), - '***', - docBlock - ].join('\n'); + let result = util.mdFence(callable.type.toString(), 'brightscript'); + if (comments.length > 0) { + result += '\n***\n' + comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); + } + return result; } public getSignatureHelpForNamespaceMethods(callableName: string, dottedGetText: string, scope: Scope): { key: string; signature: SignatureInformation }[] { From 621776cf546ed3e498f06a369262465900d7864e Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 27 Jan 2022 16:36:31 -0500 Subject: [PATCH 3/3] Use standard hover test style --- src/files/BrsFile.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 191a4f378..1fbbb0f1b 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1566,11 +1566,11 @@ describe('BrsFile', () => { let hover = file.getHover(Position.create(5, 24)); expect(hover.range).to.eql(Range.create(5, 20, 5, 29)); - expect(hover.contents).to.equal(trim` - \`\`\`brightscript - sub sayMyName(name as string) as void - \`\`\`` - ); + expect(hover.contents).to.equal([ + '```brightscript', + 'sub sayMyName(name as string) as void', + '```' + ].join('\n')); }); it('finds function hover in file scope', () => {