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

Add Snippets #55

Merged
merged 4 commits into from
Jul 29, 2019
Merged

Add Snippets #55

merged 4 commits into from
Jul 29, 2019

Conversation

corbob
Copy link
Member

@corbob corbob commented Jul 18, 2019

Fixes #51

Still need to actually create the snippets entry in Package.json.

Remove line in downloadPSES.ps1 that doesn't actually do anything ($dir is never set)
Add to ignore Snippets directory as we're pulling that from vscode-powershell.
@@ -1,5 +1,6 @@
*.zip
PowerShellEditorServices/
Snippets/
Copy link
Member

Choose a reason for hiding this comment

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

can we ship the snippets directly? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the idea of piggy-backing off of the work done for vscode-powershell, and when the package.json is updated to have the snippets in there, it should ship them with the npm...

That being said, I think we can contribute multiple snippet files, so perhaps we can include a snippets file of our own?

Copy link
Member

Choose a reason for hiding this comment

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

it makes sense to allow for multiple entries, or even custom entries in user-defined directories.
I'll poke around to see if there's a way to programmatically add snippets without populating package.json ahead of time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@yatli user-defined snippets is probably already a thing provided by coc.

I agree with @corbob that grabbing the vscode-powershell snippets is a good start here. If we want to add more on top of that, we can for sure in one of the other mechanisms

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can also create quick ps module that would be adding snippets to PowerShell.json, so users can either update it manually or add their custom snippets. Like New-VimSnippet/Remove-VimSnippet/Import-VimSnippet (similar to ISE)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know vscode has a concept of user-defined snippets, I wouldn't be surprised if coc.nvim has it as well.

That'd be the best place a New-VimSnippet could target by default.

@TylerLeonhardt
Copy link
Collaborator

@corbob do you wanna move this out of draft now? Is it ready?

@corbob
Copy link
Member Author

corbob commented Jul 26, 2019

@corbob do you wanna move this out of draft now? Is it ready?

I think it's mostly good. The only thing I think should be added is a note to the readme about needing coc-snippets.

I'll update it later tonight or tomorrow and then move it out of draft.

@corbob
Copy link
Member Author

corbob commented Jul 27, 2019

I did just notice a note that this needs nvim 0.4 or the latest vim8: https://github.com/neoclide/coc.nvim/wiki/F.A.Q#how-to-make-preview-window-shown-aside-with-pum which I've mentioned in the readme.

@corbob corbob marked this pull request as ready for review July 27, 2019 17:47
Copy link
Collaborator

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@yatli
Copy link
Member

yatli commented Jul 29, 2019

LGTM!

I've had a peek into coc-snippets, this is how it scans for snippets:

https://github.com/neoclide/coc-snippets/blob/bcfc9ecbf5e31e89a321a2f6e6bf6c3042aef166/src/textmateProvider.ts#L54-L66

So, if we modify the configuration setting snippets.textmateSnippetsRoot on activation, we make our snippets directory dynamic, and need not to prescribe it in package.json

@yatli yatli merged commit ee00661 into coc-extensions:master Jul 29, 2019
@yatli
Copy link
Member

yatli commented Jul 29, 2019

Let's merge the current version first. I'm poking around this idea and we can do it in another PR.

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.

Adding Snippets
4 participants