From fc8c8bfdc3fbdc3e192c0cd422abf465d2fb305e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 18:09:22 +0000 Subject: [PATCH 1/4] Initial plan From 2465506e3c2c2c7327d5781c09f06ecedb618ecf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 18:23:33 +0000 Subject: [PATCH 2/4] Fix hover for global functions by adding global scope fallback Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- src/Program.ts | 8 ++++- src/bscPlugin/hover/HoverProcessor.spec.ts | 39 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/Program.ts b/src/Program.ts index d50052cef..456d316dc 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -1027,11 +1027,17 @@ export class Program { let file = this.getFile(srcPath); let result: Hover[]; if (file) { + //find the scopes for this file + let scopes = this.getScopesForFile(file); + + //if there are no scopes, include the global scope so we at least get the built-in functions + scopes = scopes.length > 0 ? scopes : [this.globalScope]; + const event = { program: this, file: file, position: position, - scopes: this.getScopesForFile(file), + scopes: scopes, hovers: [] } as ProvideHoverEvent; this.plugins.emit('beforeProvideHover', event); diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts index ff0c7383e..32704625b 100644 --- a/src/bscPlugin/hover/HoverProcessor.spec.ts +++ b/src/bscPlugin/hover/HoverProcessor.spec.ts @@ -219,5 +219,44 @@ describe('HoverProcessor', () => { expect(hover?.range).to.eql(util.createRange(2, 40, 2, 50)); expect(hover?.contents).to.eql(fence('const name.sp.a.c.e.SOME_VALUE = true')); }); + + it('finds hover for global function', () => { + const file = program.setFile('source/main.brs', ` + sub main() + result = Abs(-5) + end sub + `); + program.validate(); + + // hover over the `Abs` function call + let hover = program.getHover(file.pathAbsolute, util.createPosition(2, 29))[0]; + expect(hover).to.exist; + expect(hover.range).to.eql(util.createRange(2, 29, 2, 32)); + expect(hover.contents).to.contain('function Abs(x as float) as float'); + }); + + it('finds hover for global function when file has no scopes', () => { + // Create a program with no manifest and no scopes to ensure global scope fallback + const testProgram = new Program({ rootDir: rootDir }); + + const file = testProgram.setFile('standalone/main.brs', ` + sub main() + result = Abs(-5) + end sub + `); + + // Don't validate - this simulates a file that might not be in any scopes + const scopes = testProgram.getScopesForFile(file); + + // hover over the `Abs` function call + let hover = testProgram.getHover(file.pathAbsolute, util.createPosition(2, 29))[0]; + + testProgram.dispose(); + + // After the fix, this should now work even when file has no scopes + expect(hover).to.exist; + expect(hover.range).to.eql(util.createRange(2, 29, 2, 32)); + expect(hover.contents).to.contain('function Abs(x as float) as float'); + }); }); }); From 79ebcfda8073375e64988db770f75d1cc461c7d6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 20:12:30 +0000 Subject: [PATCH 3/4] Fix hover descriptions for global functions and remove dead code Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- src/bscPlugin/hover/HoverProcessor.spec.ts | 20 ++++++++++++++- src/bscPlugin/hover/HoverProcessor.ts | 30 ++++++++++++++++++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts index 32704625b..a6b65a076 100644 --- a/src/bscPlugin/hover/HoverProcessor.spec.ts +++ b/src/bscPlugin/hover/HoverProcessor.spec.ts @@ -233,6 +233,7 @@ describe('HoverProcessor', () => { expect(hover).to.exist; expect(hover.range).to.eql(util.createRange(2, 29, 2, 32)); expect(hover.contents).to.contain('function Abs(x as float) as float'); + expect(hover.contents).to.contain('Returns the absolute value of the argument.'); }); it('finds hover for global function when file has no scopes', () => { @@ -246,7 +247,6 @@ describe('HoverProcessor', () => { `); // Don't validate - this simulates a file that might not be in any scopes - const scopes = testProgram.getScopesForFile(file); // hover over the `Abs` function call let hover = testProgram.getHover(file.pathAbsolute, util.createPosition(2, 29))[0]; @@ -257,6 +257,24 @@ describe('HoverProcessor', () => { expect(hover).to.exist; expect(hover.range).to.eql(util.createRange(2, 29, 2, 32)); expect(hover.contents).to.contain('function Abs(x as float) as float'); + expect(hover.contents).to.contain('Returns the absolute value of the argument.'); + }); + + it('finds hover for global function with both shortDescription and documentation', () => { + const file = program.setFile('source/main.brs', ` + sub main() + result = Atn(-5) + end sub + `); + program.validate(); + + // hover over the `Atn` function call + let hover = program.getHover(file.pathAbsolute, util.createPosition(2, 29))[0]; + expect(hover).to.exist; + expect(hover.range).to.eql(util.createRange(2, 29, 2, 32)); + expect(hover.contents).to.contain('function Atn(x as float) as float'); + expect(hover.contents).to.contain('Returns the arctangent (in radians) of the argument.'); + expect(hover.contents).to.contain('ATN(X)` returns "the angle whose tangent is X"'); }); }); }); diff --git a/src/bscPlugin/hover/HoverProcessor.ts b/src/bscPlugin/hover/HoverProcessor.ts index 3c2052c6d..25bdf651a 100644 --- a/src/bscPlugin/hover/HoverProcessor.ts +++ b/src/bscPlugin/hover/HoverProcessor.ts @@ -1,7 +1,7 @@ import { isBrsFile, isFunctionType, isXmlFile } from '../../astUtils/reflection'; import type { BrsFile } from '../../files/BrsFile'; import type { XmlFile } from '../../files/XmlFile'; -import type { Hover, ProvideHoverEvent } from '../../interfaces'; +import type { Hover, ProvideHoverEvent, Callable } from '../../interfaces'; import type { Token } from '../../lexer/Token'; import { TokenKind } from '../../lexer/TokenKind'; import { BrsTranspileState } from '../../parser/BrsTranspileState'; @@ -39,6 +39,32 @@ export class HoverProcessor { return parts.join('\n'); } + private buildCallableContents(callable: Callable, fence: (code: string) => string) { + const parts = [fence(callable.type.toString())]; + + // For global callables, use shortDescription and documentation properties + if (callable.shortDescription || callable.documentation) { + const docs = []; + if (callable.shortDescription) { + docs.push(callable.shortDescription); + } + if (callable.documentation) { + docs.push(callable.documentation); + } + if (docs.length > 0) { + parts.push('***', docs.join('\n\n')); + } + } else { + // For regular callables, use token-based documentation + const tokenDocs = this.getTokenDocumentation((this.event.file as BrsFile).parser.tokens, callable.functionStatement?.func?.functionType); + if (tokenDocs) { + parts.push('***', tokenDocs); + } + } + + return parts.join('\n'); + } + private getBrsFileHover(file: BrsFile): Hover | undefined { const scope = this.event.scopes[0]; const fence = (code: string) => util.mdFence(code, 'brightscript'); @@ -115,7 +141,7 @@ export class HoverProcessor { if (callable) { return { range: token.range, - contents: this.buildContentsWithDocs(fence(callable.type.toString()), callable.functionStatement?.func?.functionType) + contents: this.buildCallableContents(callable, fence) }; } } From b50df8d6e93a05fb93bd2ca3f677c67e232cd8f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 18 Jul 2025 20:33:24 +0000 Subject: [PATCH 4/4] Unify documentation handling for all callables by populating shortDescription from comment tokens Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com> --- src/bscPlugin/hover/HoverProcessor.ts | 58 ++++++--------------------- src/files/BrsFile.spec.ts | 8 +++- src/files/BrsFile.ts | 6 ++- src/globalCallables.spec.ts | 4 +- src/util.ts | 31 ++++++++++++++ 5 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/bscPlugin/hover/HoverProcessor.ts b/src/bscPlugin/hover/HoverProcessor.ts index 25bdf651a..ef2fca145 100644 --- a/src/bscPlugin/hover/HoverProcessor.ts +++ b/src/bscPlugin/hover/HoverProcessor.ts @@ -41,27 +41,19 @@ export class HoverProcessor { private buildCallableContents(callable: Callable, fence: (code: string) => string) { const parts = [fence(callable.type.toString())]; - - // For global callables, use shortDescription and documentation properties - if (callable.shortDescription || callable.documentation) { - const docs = []; - if (callable.shortDescription) { - docs.push(callable.shortDescription); - } - if (callable.documentation) { - docs.push(callable.documentation); - } - if (docs.length > 0) { - parts.push('***', docs.join('\n\n')); - } - } else { - // For regular callables, use token-based documentation - const tokenDocs = this.getTokenDocumentation((this.event.file as BrsFile).parser.tokens, callable.functionStatement?.func?.functionType); - if (tokenDocs) { - parts.push('***', tokenDocs); - } + + // Use shortDescription and documentation for all callables (both global and user-defined) + const docs = []; + if (callable.shortDescription) { + docs.push(callable.shortDescription); + } + if (callable.documentation) { + docs.push(callable.documentation); } - + if (docs.length > 0) { + parts.push('***', docs.join('\n\n')); + } + return parts.join('\n'); } @@ -151,31 +143,7 @@ export class HoverProcessor { * Combine all the documentation found before a token (i.e. comment tokens) */ private getTokenDocumentation(tokens: Token[], token?: Token) { - const comments = [] as Token[]; - if (!token) { - return undefined; - } - const idx = tokens?.indexOf(token); - if (!idx || idx === -1) { - return undefined; - } - 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; - - //any other token means there are no more comments - } else { - break; - } - } - if (comments.length > 0) { - return comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); - } + return util.getTokenDocumentation(tokens, token); } private getXmlFileHover(file: XmlFile): Hover | undefined { diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 24b613729..cb114d986 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1987,7 +1987,9 @@ describe('BrsFile', () => { ).to.equal([ '```brightscript', 'function UCase(s as string) as string', - '```' + '```', + '***', + 'Converts the string to all upper case.' ].join('\n')); }); @@ -2004,7 +2006,9 @@ describe('BrsFile', () => { '```brightscript', //TODO this really shouldn't be returning the global function, but it does...so make sure it doesn't crash right now. 'function Instr(startOrText as dynamic, textOrSubstring as string, substring? as string) as integer', - '```' + '```', + '***', + 'Returns the position of the first instances of substring within text, starting at the specified start position.\nReturns 0 if the substring is not found. Unlike the ifString.Instr() method, the first position is 1.' ].join('\n')); }); diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index d8b86cf45..536993e4b 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -616,6 +616,9 @@ export class BrsFile { functionType.addParameter(callableParam.name, callableParam.type, isOptional); } + // Extract documentation from comment tokens + const documentation = util.getTokenDocumentation(this.parser.tokens, statement.func.functionType); + this.callables.push({ isSub: statement.func.functionType.text.toLowerCase() === 'sub', name: statement.name.text, @@ -626,7 +629,8 @@ export class BrsFile { type: functionType, getName: statement.getName.bind(statement), hasNamespace: !!statement.findAncestor(isNamespaceStatement), - functionStatement: statement + functionStatement: statement, + shortDescription: documentation }); } } diff --git a/src/globalCallables.spec.ts b/src/globalCallables.spec.ts index aae6160dc..521b95993 100644 --- a/src/globalCallables.spec.ts +++ b/src/globalCallables.spec.ts @@ -63,7 +63,9 @@ describe('globalCallables', () => { ).to.eql([ '```brightscript', 'function Mid(s as string, p as integer, n? as integer) as string', - '```' + '```', + '***', + 'Returns a substring of s with length n and starting at position p.\nn may be omitted, in which case the string starting at p and ending at the end of the string is returned.\nUnlike the ifString.Mid() method, the first character in the string is position 1.' ].join('\n')); }); diff --git a/src/util.ts b/src/util.ts index db38cfd28..56b25dd7e 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1769,6 +1769,37 @@ export class Util { //just return empty string so log functions don't crash with undefined project numbers return ''; } + + /** + * Combine all the documentation found before a token (i.e. comment tokens) + */ + public getTokenDocumentation(tokens: Token[], token?: Token) { + const comments = [] as Token[]; + if (!token) { + return undefined; + } + const idx = tokens?.indexOf(token); + if (!idx || idx === -1) { + return undefined; + } + for (let i = idx - 1; i >= 0; i--) { + const currentToken = tokens[i]; + //skip whitespace and newline chars + if (currentToken.kind === TokenKind.Comment) { + comments.push(currentToken); + } else if (currentToken.kind === TokenKind.Newline || currentToken.kind === TokenKind.Whitespace) { + //skip these tokens + continue; + + //any other token means there are no more comments + } else { + break; + } + } + if (comments.length > 0) { + return comments.reverse().map(x => x.text.replace(/^('|rem)/i, '')).join('\n'); + } + } } /**