Skip to content

Feature comment docs onhover #234

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

Merged
merged 12 commits into from
May 15, 2020
32 changes: 31 additions & 1 deletion server/src/__tests__/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,35 @@ describe('findSymbolCompletions', () => {
})
})

describe('commentsAbove', () => {
it('returns a string of a comment block above a line', () => {
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
expect(analyzer.commentsAbove(CURRENT_URI, 22)).toEqual('doc for func_one')
})

it('handles line breaks in comments', () => {
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
expect(analyzer.commentsAbove(CURRENT_URI, 28)).toEqual(
'doc for func_two\nhas two lines',
)
})

it('only returns connected comments', () => {
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
expect(analyzer.commentsAbove(CURRENT_URI, 36)).toEqual('doc for func_three')
})

it('returns null if no comment found', () => {
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
expect(analyzer.commentsAbove(CURRENT_URI, 45)).toEqual(null)
})

it('works for variables', () => {
analyzer.analyze(CURRENT_URI, FIXTURES.COMMENT_DOC)
expect(analyzer.commentsAbove(CURRENT_URI, 42)).toEqual('works for variables')
})
})

describe('fromRoot', () => {
it('initializes an analyzer from a root', async () => {
const parser = await initializeParser()
Expand All @@ -210,7 +239,8 @@ describe('fromRoot', () => {

expect(connection.window.showWarningMessage).not.toHaveBeenCalled()

const FIXTURE_FILES_MATCHING_GLOB = 10
// if you add a .sh file to testing/fixtures, update this value
const FIXTURE_FILES_MATCHING_GLOB = 11

// Intro, stats on glob, one file skipped due to shebang, and outro
const LOG_LINES = FIXTURE_FILES_MATCHING_GLOB + 4
Expand Down
29 changes: 28 additions & 1 deletion server/src/__tests__/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,33 @@ describe('server', () => {
})
})

it('responds to onHover with function documentation extracted from comments', async () => {
const { connection, server } = await initializeServer()
server.register(connection)

const onHover = connection.onHover.mock.calls[0][0]

const result = await onHover(
{
textDocument: {
uri: FIXTURE_URI.COMMENT_DOC,
},
position: {
line: 17,
character: 0,
},
},
{} as any,
{} as any,
)

expect(result).toBeDefined()
expect(result).toEqual({
contents:
'Function defined on line 8\n\nthis is a comment\ndescribing the function\nhello_world\nthis function takes two arguments',
})
})

it('responds to onDocumentHighlight', async () => {
const { connection, server } = await initializeServer()
server.register(connection)
Expand Down Expand Up @@ -225,7 +252,7 @@ describe('server', () => {
)

// Limited set (not using snapshot due to different executables on CI and locally)
expect(result && 'length' in result && result.length < 5).toBe(true)
expect(result && 'length' in result && result.length < 8).toBe(true)
expect(result).toEqual(
expect.arrayContaining([
{
Expand Down
48 changes: 48 additions & 0 deletions server/src/analyser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,54 @@ export default class Analyzer {
return name
}

/**
* Find a block of comments above a line position
*/
public commentsAbove(uri: string, line: number): string | null {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if we should call this findRelatedComment. More precise as it only returns a comment for when we find a match above the function/variable.

const doc = this.uriToTextDocument[uri]

const commentBlock = []

// start from the line above
let commentBlockIndex = line - 1

// will return the comment string without the comment '#'
// and without leading whitespace, or null if the line 'l'
// is not a comment line
const getComment = (l: string): null | string => {
// this regexp has to be defined within the function
const commentRegExp = /^\s*#\s*(.*)/g
const matches = commentRegExp.exec(l)
return matches ? matches[1].trim() : null
}

let currentLine = doc.getText({
start: { line: commentBlockIndex, character: 0 },
end: { line: commentBlockIndex + 1, character: 0 },
})

// iterate on every line above and including
// the current line until getComment returns null
let currentComment: string | null = ''
while ((currentComment = getComment(currentLine))) {
commentBlock.push(currentComment)
commentBlockIndex -= 1
currentLine = doc.getText({
start: { line: commentBlockIndex, character: 0 },
end: { line: commentBlockIndex + 1, character: 0 },
})
}

if (commentBlock.length) {
// since we searched from bottom up, we then reverse
// the lines so that it reads top down.
return commentBlock.reverse().join('\n')
}

// no comments found above line:
return null
}

private getAllSymbols(): LSP.SymbolInformation[] {
// NOTE: this could be cached, it takes < 1 ms to generate for a project with 250 bash files...
const symbols: LSP.SymbolInformation[] = []
Expand Down
13 changes: 12 additions & 1 deletion server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ export default class BashServer {
return { contents: getMarkdownContent(shellDocumentation) }
}
} else {
const getCommentsAbove = (uri: string, line: number): string => {
const comment = this.analyzer.commentsAbove(uri, line)
return comment ? `\n\n${comment}` : ''
}

const symbolDocumentation = deduplicateSymbols({
symbols: this.analyzer.findSymbolsMatchingWord({
exactMatch: true,
Expand All @@ -194,9 +199,15 @@ export default class BashServer {
? `${symbolKindToDescription(symbol.kind)} defined in ${path.relative(
currentUri,
symbol.location.uri,
)}${getCommentsAbove(
symbol.location.uri,
symbol.location.range.start.line,
)}`
: `${symbolKindToDescription(symbol.kind)} defined on line ${symbol.location
.range.start.line + 1}`,
.range.start.line + 1}${getCommentsAbove(
params.textDocument.uri,
symbol.location.range.start.line,
)}`,
)

if (symbolDocumentation.length === 1) {
Expand Down
2 changes: 2 additions & 0 deletions testing/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const FIXTURE_URI = {
MISSING_NODE: `file://${path.join(FIXTURE_FOLDER, 'missing-node.sh')}`,
PARSE_PROBLEMS: `file://${path.join(FIXTURE_FOLDER, 'parse-problems.sh')}`,
SOURCING: `file://${path.join(FIXTURE_FOLDER, 'sourcing.sh')}`,
COMMENT_DOC: `file://${path.join(FIXTURE_FOLDER, 'comment-doc-on-hover.sh')}`,
}

export const FIXTURE_DOCUMENT = {
Expand All @@ -28,6 +29,7 @@ export const FIXTURE_DOCUMENT = {
MISSING_NODE: getDocument(FIXTURE_URI.MISSING_NODE),
PARSE_PROBLEMS: getDocument(FIXTURE_URI.PARSE_PROBLEMS),
SOURCING: getDocument(FIXTURE_URI.SOURCING),
COMMENT_DOC: getDocument(FIXTURE_URI.COMMENT_DOC),
}

export default FIXTURE_DOCUMENT
47 changes: 47 additions & 0 deletions testing/fixtures/comment-doc-on-hover.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env bash


# this is a comment
# describing the function
# hello_world
# this function takes two arguments
hello_world() {
echo "hello world to: $1 and $2"
}



# if the user hovers above the below hello_world invocation
# they should see the comment doc string in a tooltip
# containing the lines 4 - 7 above

hello_world "bob" "sally"



# doc for func_one
func_one() {
echo "func_one"
}

# doc for func_two
# has two lines
func_two() {
echo "func_two"
}


# this is not included

# doc for func_three
func_three() {
echo "func_three"
}


# works for variables
my_var="pizza"


my_other_var="no comments above me :("