Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Feature/better external command support#897

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
EmilyGraceSeville7cf wants to merge6 commits intobash-lsp:main
base:main
Choose a base branch
Loading
fromEmilyGraceSeville7cf:feature/better-external-command-support

Conversation

EmilyGraceSeville7cf
Copy link
Contributor

Added snippets for:

  • zip, unzip
  • find

@skovhus
Copy link
Collaborator

Thanks. There seems to be some formatting issues. Running yarn lint should fix it.

@Shane-XB-Qian
Copy link
Contributor

to be honest, those un-general chars in the label mostly would cause issues,
i would suggest you gave up those non word chars;
or actually the snip had committed in this repo looks not very good quality,
how about rollback but just keep using snip from local plugin? that mostly maintained by community perhaps better?

@EmilyGraceSeville7cf
Copy link
ContributorAuthor

EmilyGraceSeville7cf commentedJul 7, 2023
edited
Loading

@Shane-XB-Qian, can you clarify what quality issues do you mean and what extension you propose to use? I remember there was ashellman VS Code extension.

UPD:
I took a look at shellman extension. The problem with it that it doesn't support option between short/long options for some commands. But I think it'sfixable. So... my plan to enhance that extension, embrace some of our snippets and remove them from this repo when everything is done with shellman.

@skovhus
Copy link
Collaborator

how about rollback but just keep using snip from local plugin?

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.

@Shane-XB-Qian
Copy link
Contributor

Shane-XB-Qian commentedJul 17, 2023 via email

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

@skovhus
Copy link
Collaborator

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?

@EmilyGraceSeville7cf
Copy link
ContributorAuthor

or config well, then those snips looks buggy or dup.

Can you propose your suggestions on enhancing snippets? Somewhere, maybe as an issue, maybe as PR. I wanna see what you miss in snippets.

@Shane-XB-Qian
Copy link
Contributor

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.

well, this is open source, basically it's just a json file (or similar), user can modify, remove, add or just delete it.
compare to snips in server, maybe you can add a flag to disable it, but well, it looks a like tech burden? or dup?

@EmilyGraceSeville7cf
Copy link
ContributorAuthor

Please don't merge before#923. Thanks!

@EmilyGraceSeville7cfEmilyGraceSeville7cf marked this pull request as draftAugust 2, 2023 13:19
@Shane-XB-Qian
Copy link
Contributor

"sed ${1|-E,--regexp-extended|} ':${2:x} N $! b$2 ${3:command}' ${4:file}" (from#923)

$! made following expanding failed, you may need to avoid or change to another way.
// looks same case if$$.

@EmilyGraceSeville7cf
Copy link
ContributorAuthor

EmilyGraceSeville7cf commentedAug 3, 2023
edited
Loading

@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?

@Shane-XB-Qian
Copy link
Contributor

diff --git a/server/src/snippets.ts b/server/src/snippets.tsindex 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,
but since this had been merged to lsp server side, then let's try to refine it a bit.

EmilyGraceSeville7cf reacted with heart emoji

@Shane-XB-Qian
Copy link
Contributor

Shane-XB-Qian commentedOct 18, 2023
edited
Loading

@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.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@EmilyGraceSeville7cf@skovhus@Shane-XB-Qian

[8]ページ先頭

©2009-2025 Movatter.jp