-
Notifications
You must be signed in to change notification settings - Fork 132
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
Feature/better external command support #897
base: main
Are you sure you want to change the base?
Feature/better external command support #897
Conversation
Thanks. There seems to be some formatting issues. Running yarn lint should fix it. |
to be honest, those un-general chars in the label mostly would cause issues, |
@Shane-XB-Qian, can you clarify what quality issues do you mean and what extension you propose to use? I remember there was a shellman VS Code extension. UPD: |
The argument for having the snippets as part of the language server is that the usage is much wider than an extension for vscode. Bash language server is used in all editors supporting the language server protocol (which is basically all major code editors or IDEs). But if quality is a concern, then we should fix that. I’m personally barely using snippets, so I’m unaware of the best practices here. |
The argument for having the snippets as part of the language server is that the usage is much wider than an extension for vscode.
maybe wider, but not certainly better.
actually it still required some additional local plugins of that editor supported, even server had snips, but if no or not supported well or config well, then those snips looks buggy or dup.
I am not talking about vscode always, actually I am not a vscode user.
if user really wants snips, local ones maybe more *flexible* and well maintained by community by more people. (though just a wish :smile, sooner or later you would find it perhaps became mess, it's not a coding language eventually, but just some snip)
for now I only found bashls and awkls had this which both PR from Emily.
anyway, it's your project, you call.
…--
shane.xb.qian
|
Thanks for the additional context. I’m not sure I fully understand why specialized snippets for a single editor can be more flexible or better maintained than an LSP based on. Personally I’m not bothered by the bash language server snippets, but I’m curious if you would suggest having a config flag to disable them? |
Can you propose your suggestions on enhancing snippets? Somewhere, maybe as an issue, maybe as PR. I wanna see what you miss in snippets. |
well, this is open source, basically it's just a json file (or similar), user can modify, remove, add or just delete it. |
Please don't merge before #923. Thanks! |
|
@Shane-XB-Qian thanks for notifying me. I'll take a look how to fix it. UPD: In VS Code it seems to work fine. What editor do u use? |
diff --git a/server/src/snippets.ts b/server/src/snippets.ts
index 2730fc1..ff3d491 100644
--- a/server/src/snippets.ts
+++ b/server/src/snippets.ts
@@ -594,13 +594,13 @@ export const SNIPPETS: BashCompletionItem[] = [
insertText: "sed '' ${1:file}",
},
{
documentation: 'file read',
label: 'file-read',
insertText:
- "sed ${1|-E,--regexp-extended|} ':${2:x} N $! b$2 ${3:command}' ${4:file}",
+ "sed ${1|-E,--regexp-extended|} ':${2:x} N \\$! b$2 ${3:command}' ${4:file}",
},
{
documentation: 'skip first',
label: 'skip-first',
insertText: 'tail ${1|-n,-c,--lines,--bytes|} +${2:count}',
},
@@ -637,26 +637,26 @@ export const SNIPPETS: BashCompletionItem[] = [
{
documentation: 'device',
label: 'device',
insertText: '/dev/${1|null,stdin,stdout,stderr|}',
},
{
- documentation: 'completion',
- label: 'completion definition',
+ documentation: 'completion definition',
+ label: 'completion-definition',
insertText: [
'_$1_completions()',
'{',
'\treadarray -t COMPREPLY < <(compgen -W "-h --help -v --version" "\\${COMP_WORDS[1]}")',
'}',
'',
'complete -F _$1_completions ${1:command}',
].join('\n'),
},
{
- documentation: 'comment',
- label: 'comment definition',
+ documentation: 'comment definition',
+ label: 'comment-definition',
insertText: '# ${1:description}',
},
].map((item) => ({
...item,
documentation: {
value: [ a bit complex or no idea some of them, |
@EmilyGraceSeville7cf if you still wish to merge your left 3 snip PR, maybe better tidy up them (plus above my patch) again by rebase and squash and sort/review/correct etc, and some notes we've discussed, if so maybe a bit clear, and no include the dependency npm json/yaml files, or there conflicted at least. |
Added snippets for: