Community wrapper snippets

[AndreasArvidsson]

This is the Cursorless side of the community wrapper snippets

https://github.com/talonhub/community/pull/1315

Checklist

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

[AndreasArvidsson]

I think it reasonable to expect people to update community before cursorless. I know of no way of using community lists or captures in cursorless that would not show an error in the log if missing.

The ambiguity would be there if we added the command to community as well. You just cant define the same snippet phrase in both places.

I prefer the rpc solution.

[pokey]

I think it reasonable to expect people to update community before cursorless. I know of no way of using community lists or captures in cursorless that would not show an error in the log if missing.

I completely disagree. We have gone out of our way to make Cursorless talon files easy to update by supporting extensive customization. It is not uncommon for people to go months without updating community, because the fact that the only way to tweak community oftentimes is to directly change it means that upgrades can be quite painful

We should discuss a way to make Cursorless backwards compatible with community. We've done it before, but I believe that was for a new action, so we could easily wrap it in a try-catch

The ambiguity would be there if we added the command to community as well. You just cant define the same snippet phrase in both places.

Not saying it wouldn't, just pointing it out. Could be surprising if you change your function snippet in community and it doesn't update because Cursorless is preferring the Cursorless one

[AndreasArvidsson]

Doesn't that mean that we can never utilize new list or captures from community? I know of no mechanism to solve that.

Yes you can't have the same snippet in both. Personally I have totally disabled cursorless snippets. Getting around this problem might be interesting.

[pokey]

Doesn't that mean that we can never rely on new list or captures from community? I know of no mechanism to solve that.

We could add a context key that we set from community and then match on that?

Yes you cant have the same snippet in both. Personally I have totally disabled cursorless snippets. Getting around this problem might be interesting.

Yeah would be worth discussing. Esp since we ship with a conflicting snippet 🤔 https://github.com/cursorless-dev/cursorless/labels/to%20discuss

[AndreasArvidsson]

We could add a context key that we set from community and then match on that?

Unfortunately results in the same problem. If that tag is missing from community because of an older version then cursorless would error in the log.

Yeah would be worth discussing. Esp since we ship with a conflicting snippet 🤔

Agreed.

[pokey]

No not a tag a context key. Do those have to be declared?

[AndreasArvidsson]

No not a tag a context key. Do those have to be declared?

Not sure I follow. Do you mean like @mod.scope?

Probably same problem

[pokey]

Not sure. You don't explicitly declare the key for a scope in the same way as you declare a tag, so it seems possible that it won't throw an exception if undefined.

If that doesn't work, we could possibly make an onready in cursorless talon that tries to call some action (wrapped in a try-catch) to see whether it exists, and if it does, sets a tag

Anyways we can discuss if none of these options make sense to you; there's usually a way to make things work, with enough elbow grease 🙂

[pokey]

update from meet-up:

• asked aegis:

We introduced a new list in community, and would like to rely on the list in cursorless-talon. However, we expect users to upgrade cursorless-talon more frequently than community, and thus can’t assume that they have this list already. How can we leverage this list when it’s present, without things breaking when it isn’t?

• [x] @pokey will review https://github.com/cursorless-dev/cursorless/pull/1879
• [x] introduce user.cursorless_use_community_snippets tag that will enable wrapping / destinational insertion and disable cursorless snippet wrapping / insertion

[pokey]

I wonder if it's worth considering sharing the snippets from Talon to Cursorless via .cursorless/state.json. Then we could eg show them in the VSCode Cursorless side bar, handle scope-specific snippets, use them for fancy Cursorless stuff like https://github.com/cursorless-dev/cursorless/issues/446 etc

[AndreasArvidsson]

@pokey updated

[pokey]

per https://github.com/talonhub/community/pull/1305#issuecomment-1814286932, we will have cursorless own the actions / rules, and just call the actions from that comment to get what it needs about snippet. Still hide behind the use community tag

[AndreasArvidsson]

Thanks. Community updated with your suggestion

[pokey]

Ok looking good. Is there a way to test this? Ie could we activate the use_community tag for part of our tests?

[AndreasArvidsson]

Probably doable?

[pokey]

update from meet-up:

• we do want tests
• @AndreasArvidsson will keep this for now, but if time doesn't allow, @pokey will steal this PR in a week

[pokey]

There is a spoken form inconsistency here: in community, the spoken form for snippet insertion is "snip", but the default spoken form for snippet insertion in Cursorless is "snippet". So out of the box, it will be "snip funk", but "snippet funk after air". Not entirely sure how to handle that 🤔

[pokey]

Ok I added tests, but:

• See caveat in https://github.com/talonhub/community/pull/1315/files#r1471234714
• It relies on user having "code" snippet (code block). That's shipped in community, so should be fine, but we should really mock community snippet dir setting to make this more robust. Prob fine for now tho I think as long as we add a caveat somewhere

[pokey]

update from meet-up:

• run one last local test and then ship this alongside https://github.com/talonhub/community/pull/1315

[brief]

Is this not merging because of the failing test?

[pokey]

Yeah it got caught in that weird windows CI issue; totally unrelated to this PR. Just updated the branch so it gets the fix. Should pass and merge shortly