Added command history

[AndreasArvidsson]

setting:

"cursorless.commandHistory": true

Monthly log file: cursorlessCommandHistory_2023-12.jsonl

{"date":"2023-12-10","cursorlessVersion":"0.28.0-40a6fee4","command":{"version":6,"action":{"name":"setSelection","target":{"type":"primitive","modifiers":[{"type":"containingScope","scopeType":{"type":"line"}}]}},"usePrePhraseSnapshot":true}}

Voice command: "Cursorless analyze history" opens a new untitled document:

[2023-12]

Total commands: 24

Actions (7):
  setSelection: 18
  clearAndSetSelection: 1
  remove: 1
  wrapWithPairedDelimiter: 1
  getText: 1
  replace: 1
  copyToClipboard: 1

Modifiers (2):
  containingScope: 20
  relativeScope: 2

Scope types (4):
  line: 18
  surroundingPair: 2
  comment: 1
  document: 1

Checklist

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

[pokey]

Haven't looked in depth just quick comment

[AndreasArvidsson]

@pokey Updated and ready ;)

[pokey]

Looks good! Left a few comments. Also

• I think it would be nice to have at least one simple sunny-day test. You should be able to steal most of what you need from https://github.com/cursorless-dev/cursorless/blob/main/packages/cursorless-vscode-e2e/src/suite/testCaseRecorder.vscode.test.ts

[AndreasArvidsson]

Looks good! Left a few comments. Also

* [ ]  I think it would be nice to have at least one simple sunny-day test. You should be able to steal most of what you need from https://github.com/cursorless-dev/cursorless/blob/main/packages/cursorless-vscode-e2e/src/suite/testCaseRecorder.vscode.test.ts

I can't find the mechanism for mocking the file system paths.

[pokey]

I can't find the mechanism for mocking the file system paths.

The cursorless dir is already automatically set to a temp dir

cursorless/packages/cursorless-vscode/src/extension.ts

Lines 175 to 177 in 40a6fee

	const cursorlessDir = isTesting()
	? path.join(os.tmpdir(), crypto.randomBytes(16).toString("hex"))
	: path.join(os.homedir(), ".cursorless");

and then you can access it like this

cursorless/packages/cursorless-vscode-e2e/src/suite/scopeProvider/runCustomSpokenFormScopeInfoTest.ts

Lines 12 to 13 in 40a6fee

	const { scopeProvider, cursorlessTalonStateJsonPath } = (
	await getCursorlessApi()

[AndreasArvidsson]

I think it would be good hygiene to include a command id and phrase id in each log entry.

Also, it would be good to test that our sanitisation actually works. I could see that breaking without us noticing 😅

We don't really have the command id at this point in the pipeline, but I guess we could just use the constant with the knowledge that it will be correct?
What do you mean with a phrase id?

True sounds reasonable to test.

[pokey]

We don't really have the command id at this point in the pipeline, but I guess we could just use the constant with the knowledge that it will be correct?

No I just mean generate one on the fly

What do you mean with a phrase id?

Just generate an id, then use command server api to see if the phrase has changed, like we do for hat map, and if not reuse previous phrase id otherwise generate new one. Make sense?

[AndreasArvidsson]

We don't really have the command id at this point in the pipeline, but I guess we could just use the constant with the knowledge that it will be correct?

No I just mean generate one on the fly

Okay how would that id look and what would be the purpose? Why not just use the actual command id?

What do you mean with a phrase id?

Just generate an id, then use command server api to see if the phrase has changed, like we do for hat map, and if not reuse previous phrase id otherwise generate new one. Make sense?

Yes. If it's the same phrase use the same id otherwise generate new one. Using this we can group commands in the phrase.

[pokey]

Okay how would that id look and what would be the purpose? Why not just use the actual command id?

Just a uuid I guess? I thought you said we didn't have access to command id. The purpose is just hygiene. If we do transformations on the files, it's nice for each command to have an id so we can be sure it's the same. Just a data science best practice

Yes. If it's the same phrase use the same id otherwise generate new one. Using this we can group commands in the phrase.

👍

[AndreasArvidsson]

Okay how would that id look and what would be the purpose? Why not just use the actual command id?

Just a uuid I guess? I thought you said we didn't have access to command id. The purpose is just hygiene. If we do transformations on the files, it's nice for each command to have an id so we can be sure it's the same. Just a data science best practice

I don't really see how a uuid that gets generated for each command helps with anything? Or do you mean that we should just have a static one like a version number?
We don't have access to the command in the command runner itself, but we only have a single command to run Cursorless actions and that is a exported constant.

cursorless/packages/common/src/cursorlessCommandIds.ts

Lines 1 to 2 in 6718e84

	export const CURSORLESS_COMMAND_ID =
	"cursorless.command" satisfies CursorlessCommandId;

[pokey]

I don't really see how a uuid that gets generated for each command helps with anything?

It's just good practice. These IDs are very useful when doing postprocessing. But we can see if others feel strongly we should leave them out cc/ @phillco @josharian

[AndreasArvidsson]

Made a few changes

• Moved command history to the engine
• Added command history setting to ide configuration
• Added Cursorless version to the ide
• Added phrase id to command history entry
• Removed spoken command for analyzing command history
• Reference periods instead of months in the analysis code. Makes it so the we can easily change the way we slice the data in the future.

Things left

• Add tests for our sanitation code
• Split the analysis part into a separate pull request

[pokey]

update from meet-up:

• add id for each payload

[AndreasArvidsson]

id added

[pokey]

Removed analyzer; created #2161 to track re-adding. Feel free to open PR with that patch once this one is in; lmk if you want help creating the patch @AndreasArvidsson

[pokey]

Ok I finished those last two steps, and tried installing locally to make sure it works as expected. Looking good! Feel free to merge if you're happy with my changes