cursorless-dev / cursorless

Don't let the cursor slow you down
https://www.cursorless.org/
MIT License
1.13k stars 78 forks source link

Remove node references from Talon spoken form reader #2480

Closed AndreasArvidsson closed 2 months ago

AndreasArvidsson commented 2 months ago

Checklist

pokey commented 2 months ago

Can you elaborate a bit on why you've gone this route? The idea was for this to be node-specific and move it into a separate package

AndreasArvidsson commented 2 months ago

Definitely. I aim for all Cursorless features to be available in a non node environment. Since we already have a file system representation this was an easy win to share as much functionality as possible between different implementations while still supporting non node environments.

pokey commented 2 months ago

Do you have an example of a non-node environment where we might need to do this via file system?

AndreasArvidsson commented 2 months ago

Talon js for example. Can read the file via a talon action, but not via node.

But I'm not against making it more abstract.

pokey commented 2 months ago

But wouldn't talon js just get the spoken forms from talon?

AndreasArvidsson commented 2 months ago

In the long run maybe, but that would require changes to cursorless-Talon. Today the source of truth for the spoken forms is a json file and it's much easier to just read that.

Let's look at it from another angle. Why even have a file system abstraction if all file system interaction is gonna be via node?

I'm starting to lean towards actually removing the file system interface and have more custom interfaces for the editor to provide the specific data in a more environment agnostic way.

pokey commented 2 months ago

In the long run maybe, but that would require changes to cursorless-Talon. Today the source of truth for the spoken forms is a json file and it's much easier to just read that.

Yeah I'm ok with keeping the file thing in the short term if it's a lot easier, but does feel weird to use file-based rpc when you're in the same process

I'm starting to lean towards actually removing the file system interface and have more custom interfaces for the editor to provide the specific data in a more environment agnostic way.

Exactly. That's what I was proposing Tuesday

AndreasArvidsson commented 2 months ago

This latest version I like. Create engine gets the Talon spoken form as an argument and vscode has its own implementation. Then later in a separate pull request we can I remove the file system abstraction since that is used in other places as well that needs to be updated.

pokey commented 2 months ago

update from meet-up:

AndreasArvidsson commented 2 months ago

@pokey ready