Group by document and not editor

[AndreasArvidsson]

fixes #316

[pokey]

Looks good, but please add a test

[AndreasArvidsson]

Almost every action uses this code so it's already thoroughly tested. Was there any thing in particular you want to me to test?

[pokey]

To my knowledge, we have no tests that operate on two editors with the same document, which is what this PR is supposed to fix. Would be good to have a test for that

[AndreasArvidsson]

To my knowledge we can't do tests on multiple editors with our recorded tests framework. The code changes are used by almost every single action so we have regression testing. The issue with multiple editors for the same document is tested manually and I have no idea how to write a test for that.

[pokey]

Yeah you can't use test case recorder but it shouldn't be too bad. Have a look at eg https://github.com/pokey/cursorless-vscode/blob/14749f4f42b30fe8c7f7c9906ce79138982b0b2e/src/test/suite/backwardCompatibility.test.ts#L21-L36

[AndreasArvidsson]

The only way to open the same document in multiple editors is to actually open a file on disk. A bit problematic on running unit tests isn't it?

[pokey]

The only way to open the same document in multiple editors is to actually open a file on disk. A bit problematic on running unit tests isn't it?

Can just make a temp file

[AndreasArvidsson]

Of course true. I might have a look at that when I have some time over.

I'm tempted to just merge this one and then create an issue about the missing test.

[pokey]

I'm not sure I want us to get in the habit of merging PRs without tests. I made an exception for #401 because it required #160, which is out of scope for that PR. Not sure I see why a test is out of scope for this PR

[AndreasArvidsson]

@pokey Test added. I took special care to make sure that the hats are now in different editors of the same document.

[pokey]

Awesome test thank you! Left a tiny nit but otherwise looks great

[pokey]

looks good!