From 6dc709e69f78d377b54024247472f1551a94bba2 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Wed, 13 May 2020 19:00:07 -0500 Subject: [PATCH 01/12] Add feature to show comment documentation onHover --- server/src/analyser.ts | 60 ++++++++++++++++++++++++++++++++++++++++++ server/src/server.ts | 9 ++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/server/src/analyser.ts b/server/src/analyser.ts index f15f50177..8bc97c6a1 100644 --- a/server/src/analyser.ts +++ b/server/src/analyser.ts @@ -399,6 +399,66 @@ export default class Analyzer { return name } + /** + * Find a block of comments above a line position + */ + public commentsAbove(uri: string, line: number): string | null { + const doc = this.uriToTextDocument[uri] + + const comment_block = [] + + // start from the line above + let comment_block_index = line - 1 + + // check if that line starts with a comment + // its possible that some lines + // might have whitespace before the comment + // begins, so check by iterating from + // the start of the line + const starts_with_a_comment = (line: string): boolean => { + let char_index = 0 + let char = line.charAt(char_index) + while (char === ' ') { + char_index += 1 + char = line.charAt(char_index) + } + // once we reach a character that is not whitespace + // return true if its a comment, false otherwise + return char === '#' + } + + const exists_and_is_comment = (elm: string): boolean => elm !== undefined && starts_with_a_comment(elm) + + let current_line = doc.getText({ + start: { line: comment_block_index, character: 0 }, + end: { line: comment_block_index + 1, character: 0 }, + }) + + while (exists_and_is_comment(current_line)) { + // TODO: vscode must have some API for detecting comments + // should figure that out instead of hardcoding the # symbol... + current_line = current_line.replace('#', '') + if (current_line.charAt(current_line.length - 1) === '\n') { + // TODO: should preserve comment line endings? + // I think not because sometimes comment strings wrap a line + // to prevent the line from being too long, and it'd be nice + // to see it displayed as a sentence. + current_line = current_line.substr(0, current_line.length - 1) + } + comment_block.push(current_line) + comment_block_index -= 1 + current_line = doc.getText({ + start: { line: comment_block_index, character: 0 }, + end: { line: comment_block_index + 1, character: 0 }, + }) + } + + // since we searched from bottom up, we then reverse + // the lines so that it reads top down. + const comment_str = comment_block.reverse().join('\n') + return comment_str + } + 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[] = [] diff --git a/server/src/server.ts b/server/src/server.ts index 07b23ced0..7f335e73f 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -194,9 +194,16 @@ export default class BashServer { ? `${symbolKindToDescription(symbol.kind)} defined in ${path.relative( currentUri, symbol.location.uri, + )}\n\n${this.analyzer.commentsAbove( + symbol.location.uri, + symbol.location.range.start.line )}` : `${symbolKindToDescription(symbol.kind)} defined on line ${symbol.location - .range.start.line + 1}`, + .range.start.line + 1} + \n\n${this.analyzer.commentsAbove( + params.textDocument.uri, + symbol.location.range.start.line + )}`, ) if (symbolDocumentation.length === 1) { From 68a15ff3c7a578ecf951e1c5754065d745020b67 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Wed, 13 May 2020 19:14:45 -0500 Subject: [PATCH 02/12] Follow style guide --- server/src/analyser.ts | 45 +++++++++++++++++++++--------------------- server/src/server.ts | 6 +++--- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/server/src/analyser.ts b/server/src/analyser.ts index 8bc97c6a1..c96f9b415 100644 --- a/server/src/analyser.ts +++ b/server/src/analyser.ts @@ -405,58 +405,59 @@ export default class Analyzer { public commentsAbove(uri: string, line: number): string | null { const doc = this.uriToTextDocument[uri] - const comment_block = [] + const commentBlock = [] // start from the line above - let comment_block_index = line - 1 + let commentBlockIndex = line - 1 // check if that line starts with a comment // its possible that some lines // might have whitespace before the comment // begins, so check by iterating from // the start of the line - const starts_with_a_comment = (line: string): boolean => { - let char_index = 0 - let char = line.charAt(char_index) + const startsWithAComment = (line: string): boolean => { + let charindex = 0 + let char = line.charAt(charindex) while (char === ' ') { - char_index += 1 - char = line.charAt(char_index) + charindex += 1 + char = line.charAt(charindex) } // once we reach a character that is not whitespace // return true if its a comment, false otherwise return char === '#' } - const exists_and_is_comment = (elm: string): boolean => elm !== undefined && starts_with_a_comment(elm) + const existsAndIsComment = (elm: string): boolean => + elm !== undefined && startsWithAComment(elm) - let current_line = doc.getText({ - start: { line: comment_block_index, character: 0 }, - end: { line: comment_block_index + 1, character: 0 }, + let currentLine = doc.getText({ + start: { line: commentBlockIndex, character: 0 }, + end: { line: commentBlockIndex + 1, character: 0 }, }) - while (exists_and_is_comment(current_line)) { + while (existsAndIsComment(currentLine)) { // TODO: vscode must have some API for detecting comments // should figure that out instead of hardcoding the # symbol... - current_line = current_line.replace('#', '') - if (current_line.charAt(current_line.length - 1) === '\n') { + currentLine = currentLine.replace('#', '') + if (currentLine.charAt(currentLine.length - 1) === '\n') { // TODO: should preserve comment line endings? // I think not because sometimes comment strings wrap a line // to prevent the line from being too long, and it'd be nice // to see it displayed as a sentence. - current_line = current_line.substr(0, current_line.length - 1) + currentLine = currentLine.substr(0, currentLine.length - 1) } - comment_block.push(current_line) - comment_block_index -= 1 - current_line = doc.getText({ - start: { line: comment_block_index, character: 0 }, - end: { line: comment_block_index + 1, character: 0 }, + commentBlock.push(currentLine) + commentBlockIndex -= 1 + currentLine = doc.getText({ + start: { line: commentBlockIndex, character: 0 }, + end: { line: commentBlockIndex + 1, character: 0 }, }) } // since we searched from bottom up, we then reverse // the lines so that it reads top down. - const comment_str = comment_block.reverse().join('\n') - return comment_str + const commentStr = commentBlock.reverse().join('\n') + return commentStr } private getAllSymbols(): LSP.SymbolInformation[] { diff --git a/server/src/server.ts b/server/src/server.ts index 7f335e73f..35bafa65a 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -196,14 +196,14 @@ export default class BashServer { symbol.location.uri, )}\n\n${this.analyzer.commentsAbove( symbol.location.uri, - symbol.location.range.start.line + symbol.location.range.start.line, )}` : `${symbolKindToDescription(symbol.kind)} defined on line ${symbol.location .range.start.line + 1} \n\n${this.analyzer.commentsAbove( params.textDocument.uri, - symbol.location.range.start.line - )}`, + symbol.location.range.start.line, + )}`, ) if (symbolDocumentation.length === 1) { From 95f7f2d0b019d1849ef7517597c5fe758a2e0ddc Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 10:48:08 -0500 Subject: [PATCH 03/12] Adds testing fixture comment-doc-on-hover.sh --- testing/fixtures.ts | 2 ++ testing/fixtures/comment-doc-on-hover.sh | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 testing/fixtures/comment-doc-on-hover.sh diff --git a/testing/fixtures.ts b/testing/fixtures.ts index 1fdd35480..c9bb79cbd 100644 --- a/testing/fixtures.ts +++ b/testing/fixtures.ts @@ -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 = { @@ -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 diff --git a/testing/fixtures/comment-doc-on-hover.sh b/testing/fixtures/comment-doc-on-hover.sh new file mode 100644 index 000000000..94e95fcf9 --- /dev/null +++ b/testing/fixtures/comment-doc-on-hover.sh @@ -0,0 +1,18 @@ +#!/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" From 5938cc1ab63826727f5911692b310edc08e157c3 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 11:05:38 -0500 Subject: [PATCH 04/12] Updates FIXTURE_FILES_MATCHING_GLOB previously it was hardcoded at 10. It took me a while to realize that 10 referred to the number of files in the fixtures folder that ended in .sh and since I added another file: comment-doc-on-hover.sh this value of 10 needed to be updated to 11 --- server/src/__tests__/analyzer.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index 55e254f43..5069a944d 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -210,7 +210,9 @@ describe('fromRoot', () => { expect(connection.window.showWarningMessage).not.toHaveBeenCalled() - const FIXTURE_FILES_MATCHING_GLOB = 10 + // TODO: maybe not hardcode this number in case + // fixture files are added in the future? + 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 From a7ccdd3e3185d7946980a31053d1b9daade2a957 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 11:17:18 -0500 Subject: [PATCH 05/12] Updates onHover test with case for comment doc feature --- server/src/__tests__/server.test.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index 43c159d37..6083b06cc 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -67,6 +67,24 @@ describe('server', () => { {} as any, ) + // if user hovers over a function name + // the hover contents should contain + // the comment above the function definition + // see testing/fixtures/comment-doc-on-hover.sh + const commentResult = await onHover( + { + textDocument: { + uri: FIXTURE_URI.COMMENT_DOC, + }, + position: { + line: 17, + character: 0, + }, + }, + {} as any, + {} as any, + ) + expect(result).toBeDefined() expect(result).toEqual({ contents: { @@ -74,6 +92,11 @@ describe('server', () => { value: expect.stringContaining('remove directories'), }, }) + + expect(commentResult).toBeDefined() + expect(commentResult).toEqual({ + contents: expect.stringContaining('this function takes two arguments'), + }) }) it('responds to onDocumentHighlight', async () => { From 55643e7e1cc0342254b491b46852b7ad0a5f42f2 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 11:35:30 -0500 Subject: [PATCH 06/12] Adds test for commentsAbove in analyser.ts --- server/src/__tests__/analyzer.test.ts | 15 +++++++++++++++ testing/fixtures.ts | 2 +- testing/fixtures/comment-doc-on-hover.sh | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index 5069a944d..3eee07ec2 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -192,6 +192,21 @@ 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') + + expect(analyzer.commentsAbove(CURRENT_URI, 28)).toEqual( + ' doc for func_two\n has two lines', + ) + + // if there is a line break in the comments + // it should not include the above comment + expect(analyzer.commentsAbove(CURRENT_URI, 36)).not.toMatch('this is not included') + }) +}) + describe('fromRoot', () => { it('initializes an analyzer from a root', async () => { const parser = await initializeParser() diff --git a/testing/fixtures.ts b/testing/fixtures.ts index c9bb79cbd..7f71c04c7 100644 --- a/testing/fixtures.ts +++ b/testing/fixtures.ts @@ -20,7 +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')}`, + COMMENT_DOC: `file://${path.join(FIXTURE_FOLDER, 'comment-doc-on-hover.sh')}`, } export const FIXTURE_DOCUMENT = { diff --git a/testing/fixtures/comment-doc-on-hover.sh b/testing/fixtures/comment-doc-on-hover.sh index 94e95fcf9..6d630e307 100644 --- a/testing/fixtures/comment-doc-on-hover.sh +++ b/testing/fixtures/comment-doc-on-hover.sh @@ -16,3 +16,24 @@ hello_world() { # 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" +} From 35b49bf1b5c04d68e4ae9505b50a67592fd6906d Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 14:43:17 -0500 Subject: [PATCH 07/12] Changes result set length to 8 maybe a user has several executables on their path that start with 'rm', so for this test to be a bit more inclusive, we increase the result.length maximum --- server/src/__tests__/server.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index 6083b06cc..789199d41 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -248,7 +248,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([ { From dae023f790ec09587cd540bef227f562a8706a9e Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 15:36:40 -0500 Subject: [PATCH 08/12] Updates commentsAbove to strip leading whitespace --- server/src/__tests__/analyzer.test.ts | 4 +- server/src/analyser.ts | 55 ++++++++++----------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index 3eee07ec2..f939aa652 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -195,10 +195,10 @@ 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') + expect(analyzer.commentsAbove(CURRENT_URI, 22)).toEqual('doc for func_one') expect(analyzer.commentsAbove(CURRENT_URI, 28)).toEqual( - ' doc for func_two\n has two lines', + 'doc for func_two\nhas two lines', ) // if there is a line break in the comments diff --git a/server/src/analyser.ts b/server/src/analyser.ts index c96f9b415..6f84ba6bc 100644 --- a/server/src/analyser.ts +++ b/server/src/analyser.ts @@ -410,43 +410,26 @@ export default class Analyzer { // start from the line above let commentBlockIndex = line - 1 - // check if that line starts with a comment - // its possible that some lines - // might have whitespace before the comment - // begins, so check by iterating from - // the start of the line - const startsWithAComment = (line: string): boolean => { - let charindex = 0 - let char = line.charAt(charindex) - while (char === ' ') { - charindex += 1 - char = line.charAt(charindex) - } - // once we reach a character that is not whitespace - // return true if its a comment, false otherwise - return char === '#' + // 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 } - const existsAndIsComment = (elm: string): boolean => - elm !== undefined && startsWithAComment(elm) - let currentLine = doc.getText({ start: { line: commentBlockIndex, character: 0 }, end: { line: commentBlockIndex + 1, character: 0 }, }) - while (existsAndIsComment(currentLine)) { - // TODO: vscode must have some API for detecting comments - // should figure that out instead of hardcoding the # symbol... - currentLine = currentLine.replace('#', '') - if (currentLine.charAt(currentLine.length - 1) === '\n') { - // TODO: should preserve comment line endings? - // I think not because sometimes comment strings wrap a line - // to prevent the line from being too long, and it'd be nice - // to see it displayed as a sentence. - currentLine = currentLine.substr(0, currentLine.length - 1) - } - commentBlock.push(currentLine) + // 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 }, @@ -454,10 +437,14 @@ export default class Analyzer { }) } - // since we searched from bottom up, we then reverse - // the lines so that it reads top down. - const commentStr = commentBlock.reverse().join('\n') - return commentStr + 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[] { From fb307c232e4d3981a6a996ee8262ef2eb88d27e6 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 15:52:31 -0500 Subject: [PATCH 09/12] Adds more test cases for commentsAbove in analyzer --- server/src/__tests__/analyzer.test.ts | 22 ++++++++++++++++++---- testing/fixtures/comment-doc-on-hover.sh | 8 ++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index f939aa652..f11520266 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -196,14 +196,28 @@ 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', + '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') + }) - // if there is a line break in the comments - // it should not include the above comment - expect(analyzer.commentsAbove(CURRENT_URI, 36)).not.toMatch('this is not included') + 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') }) }) diff --git a/testing/fixtures/comment-doc-on-hover.sh b/testing/fixtures/comment-doc-on-hover.sh index 6d630e307..b8e5ac33b 100644 --- a/testing/fixtures/comment-doc-on-hover.sh +++ b/testing/fixtures/comment-doc-on-hover.sh @@ -37,3 +37,11 @@ func_two() { func_three() { echo "func_three" } + + +# works for variables +my_var="pizza" + + +my_other_var="no comments above me :(" + From 8905da4ec0742601d9515d20b13172cc539bc812 Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 15:56:34 -0500 Subject: [PATCH 10/12] Follows style guide --- server/src/__tests__/analyzer.test.ts | 5 ++--- server/src/analyser.ts | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/analyzer.test.ts b/server/src/__tests__/analyzer.test.ts index f11520266..f59e3dad1 100644 --- a/server/src/__tests__/analyzer.test.ts +++ b/server/src/__tests__/analyzer.test.ts @@ -201,7 +201,7 @@ describe('commentsAbove', () => { 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', + 'doc for func_two\nhas two lines', ) }) @@ -239,8 +239,7 @@ describe('fromRoot', () => { expect(connection.window.showWarningMessage).not.toHaveBeenCalled() - // TODO: maybe not hardcode this number in case - // fixture files are added in the future? + // 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 diff --git a/server/src/analyser.ts b/server/src/analyser.ts index 6f84ba6bc..0ed007aac 100644 --- a/server/src/analyser.ts +++ b/server/src/analyser.ts @@ -415,7 +415,7 @@ export default class Analyzer { // 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 commentRegExp = /^\s*#\s*(.*)/g const matches = commentRegExp.exec(l) return matches ? matches[1].trim() : null } @@ -428,7 +428,7 @@ export default class Analyzer { // iterate on every line above and including // the current line until getComment returns null let currentComment: string | null = '' - while (currentComment = getComment(currentLine)) { + while ((currentComment = getComment(currentLine))) { commentBlock.push(currentComment) commentBlockIndex -= 1 currentLine = doc.getText({ From 0b3210a33950435d740a076f149472952a4c425c Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 16:49:08 -0500 Subject: [PATCH 11/12] Adds helper function to onHover for comment-doc formatting --- server/src/server.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/src/server.ts b/server/src/server.ts index 35bafa65a..6d041cea3 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -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, @@ -194,16 +199,15 @@ export default class BashServer { ? `${symbolKindToDescription(symbol.kind)} defined in ${path.relative( currentUri, symbol.location.uri, - )}\n\n${this.analyzer.commentsAbove( + )}${getCommentsAbove( symbol.location.uri, symbol.location.range.start.line, )}` : `${symbolKindToDescription(symbol.kind)} defined on line ${symbol.location - .range.start.line + 1} - \n\n${this.analyzer.commentsAbove( - params.textDocument.uri, - symbol.location.range.start.line, - )}`, + .range.start.line + 1}${getCommentsAbove( + params.textDocument.uri, + symbol.location.range.start.line, + )}`, ) if (symbolDocumentation.length === 1) { From 1e35af14247bc5b493c9294ae52edb1b256915de Mon Sep 17 00:00:00 2001 From: nikita-skobov Date: Thu, 14 May 2020 16:49:50 -0500 Subject: [PATCH 12/12] Uses seperate test case for onHover documentation --- server/src/__tests__/server.test.ts | 32 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/server/src/__tests__/server.test.ts b/server/src/__tests__/server.test.ts index 789199d41..37bfdf672 100644 --- a/server/src/__tests__/server.test.ts +++ b/server/src/__tests__/server.test.ts @@ -67,11 +67,22 @@ describe('server', () => { {} as any, ) - // if user hovers over a function name - // the hover contents should contain - // the comment above the function definition - // see testing/fixtures/comment-doc-on-hover.sh - const commentResult = await onHover( + expect(result).toBeDefined() + expect(result).toEqual({ + contents: { + kind: 'markdown', + value: expect.stringContaining('remove directories'), + }, + }) + }) + + 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, @@ -87,15 +98,8 @@ describe('server', () => { expect(result).toBeDefined() expect(result).toEqual({ - contents: { - kind: 'markdown', - value: expect.stringContaining('remove directories'), - }, - }) - - expect(commentResult).toBeDefined() - expect(commentResult).toEqual({ - contents: expect.stringContaining('this function takes two arguments'), + contents: + 'Function defined on line 8\n\nthis is a comment\ndescribing the function\nhello_world\nthis function takes two arguments', }) })