Skip to content

Command client pre-phrase signal#676

Merged
pokey merged 7 commits intomasterfrom
command-client-pre-phrase-signal
Dec 6, 2021
Merged

Command client pre-phrase signal#676
pokey merged 7 commits intomasterfrom
command-client-pre-phrase-signal

Conversation

@pokey
Copy link
Copy Markdown
Collaborator

@pokey pokey commented Nov 27, 2021

Touches a file known to the VSCode command server before executing any phrase, to allow VSCode extensions to detect start of phrase and maintain consistency over the course of a phrase.

Inaugural use case is allowing cursorless to freeze a snapshot of the hats so that they don't shift over the course of a single command phrase (cursorless-dev/cursorless#318)

@pokey pokey changed the title Command client pre phrase signal Command client pre-phrase signal Nov 27, 2021
@pokey pokey marked this pull request as ready for review November 30, 2021 13:18
Comment thread apps/vscode/command_client/command_client.py
Comment thread apps/vscode/command_client/command_client.py Outdated
@AndreasArvidsson
Copy link
Copy Markdown
Collaborator

Overall I think it looks good

Comment thread apps/vscode/command_client/command_client.py Outdated
Comment thread apps/vscode/command_client/command_client.py Outdated

def emit_pre_phrase_signal():
"""Touches a file to indicate that a phrase is about to begin execution"""
pass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that return None would work here to make Talon think this method is implemented. Probably want a comment explaining that we're deliberately creating a no-op action, not an unimplemented action.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment to the global scope implementation. I think I'd lean towards keeping that impl; otherwise it feels like we're trying to trick Talon's static analysis

@pokey pokey mentioned this pull request Dec 6, 2021
13 tasks
@pokey pokey requested a review from rntz December 6, 2021 17:04
@pokey pokey merged commit e373780 into master Dec 6, 2021
@pokey pokey deleted the command-client-pre-phrase-signal branch December 6, 2021 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants