-
Notifications
You must be signed in to change notification settings - Fork 131
Code completion improvements #237
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
Changes from all commits
ef0d500
c62c574
01a692c
08d969a
1788538
4241bb6
a1da15e
dbf5ba2
e6f759b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ import { BashCompletionItem, CompletionItemDataType } from './types' | |
import { uniqueBasedOnHash } from './util/array' | ||
import { getShellDocumentation } from './util/sh' | ||
|
||
const PARAMETER_EXPANSION_PREFIXES = new Set(['$', '${']) | ||
|
||
/** | ||
* The BashServer glues together the separate components to implement | ||
* the various parts of the Language Server Protocol. | ||
|
@@ -100,6 +102,7 @@ export default class BashServer { | |
textDocumentSync: LSP.TextDocumentSyncKind.Full, | ||
completionProvider: { | ||
resolveProvider: true, | ||
triggerCharacters: ['$', '{'], | ||
}, | ||
hoverProvider: true, | ||
documentHighlightProvider: true, | ||
|
@@ -273,9 +276,11 @@ export default class BashServer { | |
): LSP.DocumentHighlight[] | null { | ||
const word = this.getWordAtPoint(params) | ||
this.logRequest({ request: 'onDocumentHighlight', params, word }) | ||
|
||
if (!word) { | ||
return null | ||
return [] | ||
} | ||
|
||
return this.analyzer | ||
.findOccurrences(params.textDocument.uri, word) | ||
.map(n => ({ range: n.range })) | ||
|
@@ -291,22 +296,51 @@ export default class BashServer { | |
} | ||
|
||
private onCompletion(params: LSP.TextDocumentPositionParams): BashCompletionItem[] { | ||
const word = this.getWordAtPoint(params) | ||
const word = this.getWordAtPoint({ | ||
...params, | ||
position: { | ||
line: params.position.line, | ||
// Go one character back to get completion on the current word | ||
character: Math.max(params.position.character - 1, 0), | ||
}, | ||
}) | ||
this.logRequest({ request: 'onCompletion', params, word }) | ||
|
||
if (word && word.startsWith('#')) { | ||
// Inside a comment block | ||
return [] | ||
} | ||
|
||
const currentUri = params.textDocument.uri | ||
|
||
// TODO: an improvement here would be to detect if the current word is | ||
// not only a parameter expansion prefix, but also if the word is actually | ||
// inside a parameter expansion (e.g. auto completing on a word $MY_VARIA). | ||
const shouldCompleteOnVariables = word | ||
? PARAMETER_EXPANSION_PREFIXES.has(word) | ||
: false | ||
|
||
const symbolCompletions = | ||
word === null | ||
? [] | ||
: this.getCompletionItemsForSymbols({ | ||
symbols: this.analyzer.findSymbolsMatchingWord({ | ||
exactMatch: false, | ||
word, | ||
}), | ||
symbols: shouldCompleteOnVariables | ||
? this.analyzer.getAllVariableSymbols() | ||
: this.analyzer.findSymbolsMatchingWord({ | ||
exactMatch: false, | ||
word, | ||
}), | ||
currentUri, | ||
}) | ||
|
||
if (shouldCompleteOnVariables) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this an optimization to avoid creating the massive
and if the user types anything after that, the value of start="some variable"
# user begins to type 'st':
# and hopes to autocomplete 'start'
echo ${st
# shouldCompleteOnVariables is false though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for reading through this carefully.
This is actually not an optimization. The reason for the early exit is that we only want to suggest parameters/variables here if the word that we complete on is
From my experience, this is not how language server protocol clients work. They get completion for a word (e.g. |
||
// In case we auto complete on a word that starts a parameter expansion, | ||
// we do not return anything else than variable/parameter suggestions. | ||
// Note: that LSP clients should not call onCompletion in the middle | ||
// of a word, so the following should work for client. | ||
return symbolCompletions | ||
} | ||
|
||
const reservedWordsCompletions = ReservedWords.LIST.map(reservedWord => ({ | ||
label: reservedWord, | ||
kind: LSP.SymbolKind.Interface, // ?? | ||
|
@@ -347,11 +381,6 @@ export default class BashServer { | |
] | ||
|
||
if (word) { | ||
if (word.startsWith('#')) { | ||
// Inside a comment block | ||
return [] | ||
} | ||
|
||
// Filter to only return suffixes of the current word | ||
return allCompletions.filter(item => item.label.startsWith(word)) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,5 @@ add_a_us | |
BOLD=`tput bold` # redefined | ||
|
||
echo $BOL | ||
|
||
echo "$" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable is used to test if:
This check sets
shouldCompleteOnVariables
to true only for the first two, and only if the user hasnt typed anything else. I've done some tests, and I can't find a case where${#
gets detected.