Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize file-based RPC command client beyond VSCode #956

Merged
merged 47 commits into from
Sep 10, 2022
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
423a261
split command client to applicactions
Aug 13, 2022
df2e523
Remove trailing white space
Aug 13, 2022
a345741
More white space
Aug 13, 2022
9304e22
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 13, 2022
09ef97e
Address CR comments
Aug 19, 2022
613281d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 19, 2022
2f5c726
Remove error producing pre-phrase-signal
Aug 20, 2022
dbb6893
Make VSCode Shim
Aug 21, 2022
211802a
Add Shim
Aug 21, 2022
a7c1c52
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 21, 2022
01bc65d
Remove unneeded function from tag
Aug 22, 2022
8d57e34
Merge branch 'feature/command-client-re-use' of https://github.com/Jo…
Aug 22, 2022
ce14610
Add comment to vscode
Aug 22, 2022
4c9c482
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 22, 2022
27aa1f3
fix up some types & docs
rntz Aug 24, 2022
640af19
implement emit_pre_phrase_signal on module instead of in global context
rntz Aug 24, 2022
c2fbf32
typo
rntz Aug 24, 2022
295f843
restore some lines, remove unnecessary print
rntz Aug 24, 2022
c41f842
remove unwanted changes
rntz Aug 24, 2022
7717e82
Remove duplicated code
Aug 25, 2022
340b707
Remove run_command and rename
Aug 25, 2022
2d83c9d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 25, 2022
4badb6c
Remove string from mong language
Aug 28, 2022
eb5254b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 28, 2022
00343b4
Remove string import left
Aug 28, 2022
34ae8e9
double check missing newlines
Sep 1, 2022
9fdac7e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 1, 2022
8cb4444
Remove string
Sep 1, 2022
0522403
Remove string
Sep 1, 2022
0f87908
Merge branch 'feature/command-client-re-use' of https://github.com/Jo…
Sep 1, 2022
40d8ee0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 1, 2022
4d7609f
Alter missing directory error msg
Sep 1, 2022
054e81f
Rename time out
Sep 1, 2022
c5eb3ab
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 1, 2022
a8a1409
Remove client from comment
Sep 4, 2022
5b0d283
Moved comment
Sep 4, 2022
347c9b3
Removed 'final' from comment
Sep 4, 2022
571abce
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 4, 2022
b9719f5
remove comment pre phrase from tag
Sep 4, 2022
13e7d70
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 4, 2022
39478d5
Merge branch 'feature/command-client-re-use' of https://github.com/Jo…
Sep 4, 2022
f831999
command server => rpc
Sep 4, 2022
033eae5
Merge branch 'feature/command-client-re-use' of https://github.com/Jo…
Sep 4, 2022
29afd9a
fix emit_pre_phrase_signal
rntz Sep 4, 2022
173ebef
fix "tag: command_client"
rntz Sep 4, 2022
d7e89b1
Fixes
pokey Sep 6, 2022
3d1e016
Update apps/vscode/command_client/visual_studio.py
rntz Sep 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 30 additions & 57 deletions apps/vscode/command_client/command_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
# to remove it
STALE_TIMEOUT_MS = 60_000

# The amount of time to wait for VSCode to perform a command, in seconds
VSCODE_COMMAND_TIMEOUT_SECONDS = 3.0
# The amount of time to wait for client application to perform a command, in seconds
rntz marked this conversation as resolved.
Show resolved Hide resolved
RPC_COMMAND_TIMEOUT_SECONDS = 3.0

# When doing exponential back off waiting for vscode to perform a command, how
# When doing exponential back off waiting for client application to perform a command, how
# long to sleep the first time
MINIMUM_SLEEP_TIME_SECONDS = 0.0005

Expand All @@ -26,13 +26,12 @@

mod = Module()

global_ctx = Context()
ctx = Context()
mac_ctx = Context()
linux_ctx = Context()

ctx.matches = r"""
app: vscode
tag: command_client
rntz marked this conversation as resolved.
Show resolved Hide resolved
"""
mac_ctx.matches = r"""
os: mac
Expand All @@ -49,11 +48,8 @@ def __repr__(self):
return "<argument not set>"


def run_vscode_command_by_command_palette(command_id: str):
"""Execute command via command palette. Preserves the clipboard."""
actions.user.command_palette()
actions.user.paste(command_id)
actions.key("enter")
class NoFileServerException(Exception):
pass


def write_json_exclusive(path: Path, body: Any):
Expand Down Expand Up @@ -123,22 +119,23 @@ def handle_existing_request_file(path):
robust_unlink(path)


def run_vscode_command(
def run_command(
command_id: str,
*args: str,
*args,
wait_for_finish: bool = False,
return_command_output: bool = False,
):
"""Runs a VSCode command, using command server if available
"""Runs a command, using command server if available

Args:
command_id (str): The ID of the VSCode command to run
command_id (str): The ID of the command to run.
args: The arguments to the command.
wait_for_finish (bool, optional): Whether to wait for the command to finish before returning. Defaults to False.
return_command_output (bool, optional): Whether to return the output of the command. Defaults to False.

Raises:
Exception: If there is an issue with the file-based communication, or
VSCode raises an exception
application raises an exception

Returns:
Object: The response from the command, if requested.
Expand All @@ -152,9 +149,7 @@ def run_vscode_command(
if not communication_dir_path.exists():
if args or return_command_output:
raise Exception("Must use command-server extension for advanced commands")
print("Communication dir not found; falling back to command palette")
run_vscode_command_by_command_palette(command_id)
return
raise NoFileServerException("Communication directory not found")

request_path = communication_dir_path / "request.json"
response_path = communication_dir_path / "response.json"
Expand All @@ -180,9 +175,9 @@ def run_vscode_command(
print("WARNING: Found old response file")
robust_unlink(response_path)

# Then, perform keystroke telling VSCode to execute the command in the
# request file. Because only the active VSCode instance will accept
# keypresses, we can be sure that the active VSCode instance will be the
# Then, perform keystroke telling client application to execute the command in the
# request file. Because only the active client application instance will accept
# keypresses, we can be sure that the active client application instance will be the
# one to execute the command.
actions.user.trigger_command_server_command_execution()

Expand Down Expand Up @@ -221,7 +216,7 @@ def get_communication_dir_path():
if hasattr(os, "getuid"):
suffix = f"-{os.getuid()}"

return Path(gettempdir()) / f"vscode-command-server{suffix}"
return Path(gettempdir()) / f"{actions.user.command_server_directory()}{suffix}"


def robust_unlink(path: Path):
Expand Down Expand Up @@ -261,7 +256,7 @@ def read_json_with_timeout(path: str) -> Any:
Returns:
Any: The json-decoded contents of the file
"""
timeout_time = time.perf_counter() + VSCODE_COMMAND_TIMEOUT_SECONDS
timeout_time = time.perf_counter() + RPC_COMMAND_TIMEOUT_SECONDS
sleep_time = MINIMUM_SLEEP_TIME_SECONDS
while True:
try:
Expand Down Expand Up @@ -289,28 +284,16 @@ def read_json_with_timeout(path: str) -> Any:

@mod.action_class
class Actions:
def vscode(command_id: str):
"""Execute command via vscode command server, if available, or fallback
to command palette."""
run_vscode_command(command_id)

def vscode_and_wait(command_id: str):
"""Execute command via vscode command server, if available, and wait
for command to finish. If command server not available, uses command
palette and doesn't guarantee that it will wait for command to
finish."""
run_vscode_command(command_id, wait_for_finish=True)

def vscode_with_plugin(
def run_rpc_command(
command_id: str,
arg1: Any = NotSet,
arg2: Any = NotSet,
arg3: Any = NotSet,
arg4: Any = NotSet,
arg5: Any = NotSet,
):
"""Execute command via vscode command server."""
run_vscode_command(
"""Execute command via application command server."""
rntz marked this conversation as resolved.
Show resolved Hide resolved
run_command(
command_id,
arg1,
arg2,
Expand All @@ -319,16 +302,16 @@ def vscode_with_plugin(
arg5,
)

def vscode_with_plugin_and_wait(
def run_rpc_command_and_wait(
rntz marked this conversation as resolved.
Show resolved Hide resolved
command_id: str,
arg1: Any = NotSet,
arg2: Any = NotSet,
arg3: Any = NotSet,
arg4: Any = NotSet,
arg5: Any = NotSet,
):
"""Execute command via vscode command server and wait for command to finish."""
run_vscode_command(
"""Execute command via application command server and wait for command to finish."""
run_command(
command_id,
arg1,
arg2,
Expand All @@ -338,16 +321,16 @@ def vscode_with_plugin_and_wait(
wait_for_finish=True,
)

def vscode_get(
def run_rpc_command_get(
command_id: str,
arg1: Any = NotSet,
arg2: Any = NotSet,
arg3: Any = NotSet,
arg4: Any = NotSet,
arg5: Any = NotSet,
) -> Any:
"""Execute command via vscode command server and return command output."""
return run_vscode_command(
"""Execute command via application command server and return command output."""
return run_command(
command_id,
arg1,
arg2,
Expand All @@ -357,14 +340,14 @@ def vscode_get(
return_command_output=True,
)

def command_server_directory() -> str:
"""Return the final directory of the command server"""
rntz marked this conversation as resolved.
Show resolved Hide resolved

def trigger_command_server_command_execution():
"""Issue keystroke to trigger command server to execute command that
was written to the file. For internal use only"""
actions.key("ctrl-shift-f17")

def emit_pre_phrase_signal() -> bool:
"""Touches a file to indicate that a phrase is about to begin execution"""

def did_emit_pre_phrase_signal() -> bool:
"""Indicates whether the pre-phrase signal was emitted at the start of this phrase"""
return did_emit_pre_phrase_signal
Expand All @@ -382,15 +365,6 @@ def trigger_command_server_command_execution():
actions.key("ctrl-shift-alt-p")


@global_ctx.action_class("user")
class GlobalUserActions:
def emit_pre_phrase_signal() -> bool:
# NB: We explicitly define a noop version of this action in the global
# context here so that it doesn't do anything before phrases if you're not
# in vscode.
return False


@ctx.action_class("user")
class UserActions:
def emit_pre_phrase_signal() -> bool:
Expand Down Expand Up @@ -427,7 +401,6 @@ def get_signal_path(name: str) -> Path:
def pre_phrase(_: Any):
try:
global did_emit_pre_phrase_signal

did_emit_pre_phrase_signal = actions.user.emit_pre_phrase_signal()
except MissingCommunicationDir:
pass
Expand Down
21 changes: 21 additions & 0 deletions apps/vscode/command_client/command_client_tag.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from talon import Module

mod = Module()

mod.tag(
"command_client", desc="For applications which implement File based RPC with Talon"
)


@mod.action_class
class Actions:
def command_server_directory() -> str:
"""The dirctory which contains the files required for communication between the application and Talon.
This is the only function which absolutely must be implemented for any application using the command-client."""

def emit_pre_phrase_signal() -> bool:
"""The command client can touch a signal file at the start of a phrase. If your
implementation does not require this, override emit_pre_phrase_signal to
return False."""
# Unless we're in a command client app, we do nothing.
return False
15 changes: 15 additions & 0 deletions apps/vscode/command_client/visual_studio.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from talon import Context, actions

ctx = Context()

ctx.matches = r"""
app: visual_studio
"""

ctx.tags = ["user.command_client"]


@ctx.action_class("user")
class VisualStudioActions:
def command_server_directory() -> str:
return "visual-studio-commandServer"
rntz marked this conversation as resolved.
Show resolved Hide resolved
92 changes: 92 additions & 0 deletions apps/vscode/command_client/vscode.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
from pickle import FALSE
from typing import Any

from talon import Context, Module, actions

from .command_client import NoFileServerException, NotSet, run_command

ctx = Context()

ctx.matches = r"""
app: vscode
"""
ctx.tags = ["user.command_client"]
mod = Module()


def command_server_or_client_fallback(command_id: str, wait: bool):
"""Execute command via command server and us command pallete if directory not present."""
try:
run_command(command_id, wait_for_finish=wait)
except NoFileServerException:
actions.user.command_palette()
actions.user.paste(command_id)
actions.key("enter")
print(
"Command issues via command palette for better performance install the VSCode extension for Talon"
)


@ctx.action_class("user")
class VsCodeAction:
def command_server_directory() -> str:
return "vscode-command-server"


# These commands are shims, to provide backwards compatibility, they may be removed in the fuuture.
# Prefer the run_command... version in command_client.
rntz marked this conversation as resolved.
Show resolved Hide resolved
@mod.action_class
class Actions:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened to the pre-phrase signal code? Unless I've missed something, it appears to have been dropped

Copy link
Collaborator

@pokey pokey Sep 4, 2022

Choose a reason for hiding this comment

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

Ah nvm I see it now. A bit confusing how it's declared in multiple places

What's the purpose of allowing applications to turn it off? Seems simpler to just leave it on and let all applications share the same impl

Choose a reason for hiding this comment

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

See command client 368 its in ctx which uses the match belwo to check we are in the correct context. I put a comment in command_client_tag to explain when to overwrite

ctx.matches = r"""
tag: command_client
"""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove it from the actions so that it cannot be turned of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Uh, this seems to have been changed into a non-working state. There's no module which declares emit_pre_phrase_signal, only a context which implements it, which should be producing an error in the talon log. We probably want the default implementation to be return False. There's no sense touching a file if we're not in a command client. I'll go change this now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, this should be working now. @pokey is this okay? Did you want us to always touch the pre-phrase file, even if we don't have a command-client app focused?

Copy link
Collaborator

Choose a reason for hiding this comment

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

interestingly, it doesn't seem to be causing an error in the talon log. not sure why.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok. it does error if I actually try to use the action, though. I'll try to minimize that and report it as a talon bug.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, I've reported the underlying bug to talon, talonvoice/talon#525.

def vscode(command_id: str):
"""Execute command via vscode command server, if available, or fallback
to command palette."""
command_server_or_client_fallback(command_id, False)

def vscode_and_wait(command_id: str):
"""Execute command via vscode command server, if available, and wait
for command to finish. If command server not available, uses command
palette and doesn't guarantee that it will wait for command to
finish."""
command_server_or_client_fallback(command_id, True)

def vscode_with_plugin(
command_id: str,
arg1: Any = NotSet,
arg2: Any = NotSet,
arg3: Any = NotSet,
arg4: Any = NotSet,
arg5: Any = NotSet,
):
"""Execute command via vscode command server."""
actions.user.run_command(
command_id,
arg1,
arg2,
arg3,
arg4,
arg5,
)

def vscode_with_plugin_and_wait(
command_id: str,
arg1: Any = NotSet,
arg2: Any = NotSet,
arg3: Any = NotSet,
arg4: Any = NotSet,
arg5: Any = NotSet,
):
"""Execute command via vscode command server and wait for command to finish."""
actions.user.run_rpc_command_and_wait(command_id, arg1, arg2, arg3, arg4, arg5)

def vscode_get(
command_id: str,
arg1: Any = NotSet,
arg2: Any = NotSet,
arg3: Any = NotSet,
arg4: Any = NotSet,
arg5: Any = NotSet,
) -> Any:
"""Execute command via vscode command server and return command output."""
return actions.user.run_rpc_command_get(
command_id, arg1, arg2, arg3, arg4, arg5
)