Remove node references from Talon spoken form reader

[AndreasArvidsson]

Checklist

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

[pokey]

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]

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]

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

[AndreasArvidsson]

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]

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

[AndreasArvidsson]

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]

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]

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]

update from meet-up:

• move json reader to a new package called file-system-common

[AndreasArvidsson]

@pokey ready

[pokey]

looks good but just want to make sure you had a chance to try this locally; see comment below

[pokey]

👍