Skip to content
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

Adds support for function docs in hover #495

Merged
merged 5 commits into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions src/LanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was forcing all hovers to be brightscript...but we need to support any documentation information provided, so force the underlying files to provide the full markdown text instead of casting it all here.

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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,9 @@ export class Program {
if (!file) {
return null;
}
const hover = file.getHover(position);

return Promise.resolve(file.getHover(position));
return Promise.resolve(hover);
}

/**
Expand Down
75 changes: 71 additions & 4 deletions src/files/BrsFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
35 changes: 32 additions & 3 deletions src/files/BrsFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,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);

Expand Down Expand Up @@ -1452,15 +1453,15 @@ export class BrsFile {
return {
range: token.range,
//append the variable name to the front for scope
contents: typeText
contents: fence(typeText)
};
}
}
for (const labelStatement of functionScope.labelStatements) {
if (labelStatement.name.toLocaleLowerCase() === lowerTokenText) {
return {
range: token.range,
contents: `${labelStatement.name}: label`
contents: fence(`${labelStatement.name}: label`)
};
}
}
Expand All @@ -1475,13 +1476,41 @@ 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;

//any other token means there are no more comments
} else {
break;
}
}
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 }[] {
if (!dottedGetText) {
return [];
Expand Down
8 changes: 8 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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```';
}

}

/**
Expand Down