cursorless
cursorless copied to clipboard
cursorless-dev
Add cursorless symbols to cursorless
Since there is symmetry between the cursorless commands and these symbols it makes sense to define them in the same repository.
Copied these from pokey_talon
Checklist
- [ ] I have added tests
- [ ] I have updated the docs and cheatsheet
- [ ] I have not broken the cheatsheet
@pokey suggested that they should be configurable using the csv overrides. I agree but I'm not sure how to approach that. If you have some suggestions how to approach this problem, please share and I'll be glad to spend some time on it.
This might not be the cleanest approach, but it seemed to functionally work
yeah this impl is ~fine, tho I'd be tempted to insert using a snippet 🤔
but there are bigger questions around whether we want this stuff to be included in Cursorless at all or if it should be in community, as mentioned in https://github.com/cursorless-dev/cursorless-talon/pull/162#issuecomment-1904038995
might make sense to drop into a meet-up to discuss?
Yep! Reiterating here for visibility: the main argument for me that putting these into cursorless makes sense is that this is where those delimiters are defined for cursorless itself. There is symmetry between the names used here and the symbols. If we defined it in community instead, it would be awkward to keep in sync as cursorless evolves and it would be questionable to use the user's custom abbreviations for these since they are in cursorless configuration which wouldn't exist for all community users.
yeah if we moved them to community we'd want to move customization of paired delimiters there as well and then use those for the Cursorless scopes / wrappers as well, similar to the way we let community define the user.any_alphanumeric_key capture that we use for decorated marks (eg the "air" in "take air")
Gotcha. One approach could be to adopt this temporarily and move it later if we decide to, since this is closer to current configuration, I think?
We discussed this PR in the meet-up on Sunday. I think we all agree that this approach is not the long-term solution we want, but that doesn't necessarily mean that this approach is a bad short-term solution. Fwiw, here is the long-term solution we landed on: https://github.com/talonhub/community/issues/546#issuecomment-1914536331.
When deciding whether to accept this PR, the questions are:
- How long will it be before we get to the long-term solution we desire?
- How much benefit is there to this PR in the interim?
- How much damage is there from this PR in the interim?
- How hard will it be to roll this PR back when we move to the long-term solution?
How long will it be before we get to the long-term solution we desire?
This could be a while, as the migration path is going to require a bit of work
How much benefit is there to this PR in the interim?
The primary benefit is that a user can watch one of my videos, see me saying "round", and have that work out of the box. However:
- This command will only work in VSCode
- Dozens of other commands that I use won't be available. The real solution here is https://github.com/talonhub/community/issues/1221 and https://github.com/cursorless-dev/cursorless/discussions/547
So while it's clear that there is a short-term benefit here, it feels like it's addressing a tiny fraction of a much larger problem, in a way that doesn't move us towards the longer term goals
How much damage is there from this PR in the interim?
The biggest risk from our perspective is that the fact that it only works in VSCode will encourage users to either copy this code, or fork and modify Cursorless, in order to get it to work elsewhere. We strongly discourage using internal Cursorless captures / actions, and we also discourage forking Cursorless, as we want the upgrade path to be simpler. So no matter how they proceed, we've induced them to make Cursorless harder to support. The only desirable way for them to get these things working outside of VSCode is for them to modify their community wrapper commands, which is what they'd have to do anyway today.
How hard will it be to roll this PR back when we move to the long-term solution?
Probably not too bad. We could fairly easily disable these commands when we detect the tag that indicates that they've upgraded to the version of community that includes the new pairs
Conclusion
Based on the trade-off of benefit vs damage, from our perspective, the benefit doesn't outweigh the potential long-term damage of encouraging users to fork Cursorless or use our internal APIs
All that being said, opening a PR is probably the best way to get us to take a topic seriously, so from that perspective this PR has been a success 😊
I'll leave the PR open in case you want to make a case for it, but feel free to close if you agree
I'll respond in more depth later, but for now I'll start by sharing this PR in response: https://github.com/talonhub/community/pull/1375
I disagree with the cost benefit analysis above. I think the benefits outweigh the costs significantly, especially when you consider it from a systemic perspective.
The biggest risk from our perspective is that the fact that it only works in VSCode will encourage users to either copy this code, or fork and modify Cursorless, in order to get it to work elsewhere.
I'm not sure I understand why people would do this. I've been a vim user for twenty years, an emacs user for about a year and a half, and when I tried to use cursorless I found the friction in using any other editor besides vs code too high to warrant me continuing to use my editors of choice. I don't think that this changes that calculus. The suggested workaround is this:
modify their community wrapper commands
I submitted a pull request to do this for the community so that people would not have to do this on their own. I'll be interested to see how the response is.
I want to speak to the benefit here a little bit. To me the benefit of reducing the difference between vanilla cursorless install and what is shown in the videos is an incredible difference in ease of adoption for new users. As someone who has just gone through this phase I found it very difficult. I'm still struggling to get all of the functionality in place that I want. I'm a long way from having the videos working because I don't want to make a fork of the community repository. The cost of making everyone keep their own fork is that it reduces the incentive for anyone to contribute back to the community repository. Over time this is a drag that is going to be very expensive for the community repository. Any short term fix that reduces the likelihood that someone will fork the repository is a benefit that will continue to pay dividends as long as that person is a user of talon since it reduces the cost for them to contribute back. I think that is worthwhile.
I would argue for closing this PR in favour of the approach proposed in https://github.com/talonhub/community/issues/546#issuecomment-1914536331