Pass snippets as argument to engine

[AndreasArvidsson]

Makes it so that Cursorless everywhere editors doesn't have to provide a snippet implementation

Checklist

• [/] I have added tests
• [/] I have updated the docs and cheatsheet
• [/] I have not broken the cheatsheet

[AndreasArvidsson]

I was thinking that since snippets is probably going to be deprecated this was a reasonable decision. When we make the next version we can do it more properly. But right now I'm blocked on this and I don't think any other editor is going to want to implement our json based snippets in the coming few weeks?

This is just about getting rid of node references in engine. A much better interface for snippets could definitely be designed.

With that said let me have another look at it tomorrow and I might come up with a solution you like better :)

[AndreasArvidsson]

I've had another look and started a more more abstracted implementation and bailed. Basically most of what is in Snippets.ts that I here reamed to VscodeSnippets.ts is vscode specific. The fact that we are polling the file system. The handling of directory error message. The fact that you can a register third party snippets to some extent. If I remove everything that isn't vscode and make an abstraction it basically becomes more code for no good reason. I stand by my original assessment that this should probably live in vscode until we make a refactor where we used the new snippet format and don't poll.

[AndreasArvidsson]

Implementations like this also makes it a bit hard to abstract snippets. We are explicitly creating a file in the file system and then opening in the editor. To open the file we need a filepath so we are still stuck in the file system with snippets. Here my inclination is just to drill the file system instance through and add touch to that. We can later on revamp the entire snippet system and make its smarter, but for now I can a just need to be unblocked. Sounds reasonable? https://github.com/cursorless-dev/cursorless/blob/2e69df8b9b8852a1df6cba5a6b5b7a73d82d6c63/packages/cursorless-engine/src/actions/GenerateSnippet/openNewSnippetFile.ts#L20

[pokey]

Implementations like this also makes it a bit hard to abstract snippets. We are explicitly creating a file in the file system and then opening in the editor. To open the file we need a filepath so we are still stuck in the file system with snippets. Here my inclination is just to drill the file system instance through and add touch to that. We can later on revamp the entire snippet system and make its smarter, but for now I can a just need to be unblocked. Sounds reasonable?

https://github.com/cursorless-dev/cursorless/blob/2e69df8b9b8852a1df6cba5a6b5b7a73d82d6c63/packages/cursorless-engine/src/actions/GenerateSnippet/openNewSnippetFile.ts#L20

Wouldn't my second suggestion above solve this problem? Ie generate snippet, insert snippet and wrap with snippet are just custom actions passed into the engine. If this isn't making sense let's discuss tomorrow alongside vscode vs engine thing

[AndreasArvidsson]

That sounds like a reasonable solution, but to be fair I don't really have the energy for that right now. This was only moving a single file that will unblock me. And we can then later make a more flexible approach. Happy to discuss it tomorrow.

[pokey]

Let's discuss tomorrow

[pokey]

update from meet-up: pokey caved

[AndreasArvidsson]

Done