Skip to content
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

Pr snip not work #46

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Shane-XB-Qian
Copy link

closes #44

@Beaglefoot
Copy link
Owner

Looks good. Could you please remove 3 lines mentioned in other comment? And I'll merge.

@Shane-XB-Qian
Copy link
Author

remove 3 lines mentioned in other comment

what's that? i did not get you.. 😄

@Shane-XB-Qian
Copy link
Author

you meant not to change snip dict's title which in snippets.json, those 3 lines?


detail: info.description,
insertText: info.body.join('\n'),
insertTextFormat: InsertTextFormat.Snippet,
Copy link
Owner

Choose a reason for hiding this comment

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

These 3 lines are unnecessary and added on CompletionResolve event with enrichWithSnippetDetails function.

Copy link
Author

Choose a reason for hiding this comment

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

actually have to, otherwise those are not snip items.
those are working for auto completion, vs compared to completionResolve looks those for manual omni completion.

@Beaglefoot
Copy link
Owner

remove 3 lines mentioned in other comment

what's that? i did not get you.. smile

Totaly forgot to submit review ;)

@Shane-XB-Qian
Copy link
Author

and this pr is just trying to solve auto completion for snip from snippets.json
but general func/var etc items looks you had to make them to be generated automatically also.
// and/or as #45 said, those/you generated items format seems cannot be recognized actually.
// and seems you did not impl trigger char feature.

@Shane-XB-Qian
Copy link
Author

and did not impl textDocument/signatureHelp feature too. 😄

@Shane-XB-Qian
Copy link
Author

Shane-XB-Qian commented Apr 17, 2023

update: the auto-compl cannot work perhaps just #45 the generated fmt was not recognized.
// just a guess, no way/yet to verify it.

@Shane-XB-Qian
Copy link
Author

@Beaglefoot
perhaps either merge this,
or give up snip from lsp server side (rollback that previous snip commit), mostly local may have snip plugin already, that's some kinds of dup.
// anyway, #45 should be a good enhance when you have time to refine.

@Shane-XB-Qian
Copy link
Author

please refer bash-lsp/bash-language-server#897 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

snip not work
2 participants