Force tab size on test run

[Will-Sommers]

• This fixes test failures related to different local configurations
• Tests expect tabSize to be 4

Fixes: #598

Checklist

• N/A - I have added tests

[AndreasArvidsson]

This will only force tab size to be for when re-running the tests. I think we need to add the same to the test recorder otherwise every test will be recorded with the user tab setting, but run with a fixed setting.

[pokey]

This will only force tab size to be for when re-running the tests. I think we need to add the same to the test recorder otherwise every test will be recorded with the user tab setting, but run with a fixed setting.

I'll second what @AndreasArvidsson says here. I think if we detect that the extension is running in debug mode, we should make the same tabSize change

[Will-Sommers]

Not important, but I'm still a little curious about why this didn't break for me, with other tests that I've recorded.

@pokey — I'll likely add the setting here as well rather than in the actual extension.

https://github.com/cursorless-dev/cursorless/blob/main/src/test/suite/recorded.test.ts#L74-L78

[pokey]

Not important, but I'm still a little curious about why this didn't break for me, with other tests that I've recorded.

did you record tests that include snippet wrapping? I think those are the ones that will break

@pokey — I'll likely add the setting here as well rather than in the actual extension.

https://github.com/cursorless-dev/cursorless/blob/main/src/test/suite/recorded.test.ts#L74-L78

Sorry not sure I follow—that won't impact recording right?

[Will-Sommers]

did you record tests that include snippet wrapping? I think those are the ones that will break

Nope, you're right. One other indent and fold test broke as well though

Sorry not sure I follow—that won't impact recording right?

Multi-tasking between emails, sorry. You're right.

[pokey]

looks good with minor tweak

[pokey]

hmm maybe we should only do this if they're recording a test? Otherwise we'll clobber their tab size if they have cursorless debug mode on

[Will-Sommers]

Heyo @pokey — I moved this entire step into the TestCaseRecorder since we have state there about whether we should or should not set the tabs. That also bring the code out of the CommandRunner loop. We clean up at the end and reset to the original tab setting, if a user stops recording their test.

[pokey]

I think we should do this one in the preCommandHook / errorHook / postCommandHook so that

1. The code should be simpler, I think, because we don't need to set / unset it in so many places
2. We can properly handle splits in the future. Your current code will break with multiple splits

I would a keep list of visible editors and their original tab stops in preCommandHook, and reset them on those editors in postCommandHook / errorHook

Does that make sense?

Alternately, you can just do activeTextEditor, if the visible editors thing looks painful. I am considering adding split testing support in #721; debating whether it's worth supporting recording tests this way. So if it looks painful to do all visible editors, active is prob fine, but I still think it's simpler to do it in pre / postCommandHook.

But I don't feel too strongly if you think it's going to be better this way 😊

[AndreasArvidsson]

@pokey I think you can have a look at this one now

[pokey]

Looks good to me! Needed to make some tweaks because the restore code was broken