fix(injection): send newlines as Return on the xdotool backend (#962)
* fix(injection): send newlines as Return on the xdotool backend xdotool type maps '\n' to the Linefeed keysym, which GTK targets silently drop — "new line" produced nothing on X11 even though the segment reached the injector. Split each chunk on '\n' and press Return between typed segments so a newline lands as a real keypress in every app. Verified against a live GtkTextView on Xvfb: the same "A\nB" input that previously produced 'AB' now produces 'A\nB'. Fixes #955 * fix(commands): consume trailing punctuation after mid-utterance commands "New paragraph." inside a longer utterance substituted correctly but left the engine's sentence-final period typed after the break ("\\n\\n. Does this work now?"). Text-command substitution now consumes a run of trailing , . ! ? ; : … when it precedes whitespace or the end of the segment — the reporter's comma-phrased case ("…, new paragraph, …") stops leaking the stray comma the same way. A mark glued to following text ("newline.io") is untouched. Fixes #955 * fix(injection,commands): resume mid-chunk on retry, don't eat produced marks Greptile review follow-ups on #962: - A failed Return mid-chunk left 'typed' at the chunk start, so the retry re-sent text xdotool had already delivered. 'typed' now advances per typed segment and per Return keypress, so the retry resumes exactly at the un-typed position — and the clipboard fallback reports the same progress if retries run out. - _TRAILING_PUNCTUATION could consume a mark produced by an earlier command replacement ('hello new line exclamation mark' gave '\\n' instead of '\\n!'). The eater now only consumes marks before whitespace; the end-of-utterance case ('hello new line.') is a pre-pass on the raw segment before any substitution runs. * style: black-format comment indent and test condition * fix(commands): strip segment before the end-of-utterance cleanup Trailing whitespace ('hello new line. ') hid the final mark from the pre-pass: the raw segment ended in a space, and the stripped segment left the dot at end-of-string where the mid-utterance eater cannot touch it. Strip first, then run the cleanup. Also add the missing type hints on the new test helper. --------- Co-authored-by: Jatin K Malik <jatinkrmalik@gmail.com>
This commit is contained in:
devin-ai-integration[bot]
co-authored by
Jatin K Malik
Jatin K Malik
parent
e866cb85e4
commit
d3ba4ee743
@@ -27,6 +27,14 @@ _ATTACH_LEFT_WS = r"[^\S\n]*"
|
||||
# phrases — the same marks stay untouched on real text.
|
||||
_EDGE_PUNCTUATION = ".,!?;:…"
|
||||
|
||||
# Trailing punctuation right after a spoken command mid-utterance is also
|
||||
# phrasing, not text ("testing, new paragraph, done" must not type a stray
|
||||
# comma after the break). Command substitution consumes it — but only
|
||||
# before whitespace: a mark at end-of-string may have been produced by an
|
||||
# earlier command replacement ("…new line exclamation mark" → "\n!"), and
|
||||
# the end-of-utterance case is handled by a pre-pass on the raw segment.
|
||||
_TRAILING_PUNCTUATION = r"(?:[,.!?;:…]+(?=\s))?"
|
||||
|
||||
# Base English text commands (always present).
|
||||
_BASE_TEXT_COMMANDS = {
|
||||
# Line commands
|
||||
@@ -119,7 +127,8 @@ class CommandProcessor:
|
||||
"""Compile regex patterns for command matching."""
|
||||
# Longer phrases first so alternation prefers "punto interrogativo" over "punto"
|
||||
text_keys = sorted(self.text_commands.keys(), key=len, reverse=True)
|
||||
text_cmd_pattern = r"\b(" + "|".join(re.escape(cmd) for cmd in text_keys) + r")\b"
|
||||
self._text_cmd_alternation = "|".join(re.escape(cmd) for cmd in text_keys)
|
||||
text_cmd_pattern = r"\b(" + self._text_cmd_alternation + r")\b"
|
||||
self.text_cmd_regex = re.compile(text_cmd_pattern, re.IGNORECASE)
|
||||
|
||||
# Create regex pattern for action commands
|
||||
@@ -267,6 +276,18 @@ class CommandProcessor:
|
||||
"", self.action_cmd_regex.sub("", self.text_cmd_regex.sub("", core))
|
||||
)
|
||||
segment_text = core if core and not residual.strip() else text
|
||||
|
||||
# End-of-utterance form of _TRAILING_PUNCTUATION: a segment that
|
||||
# ends with a command plus engine marks ("hello new line.") has
|
||||
# no following whitespace for the eater to see, so strip the run
|
||||
# here. Runs at end-of-string mid-loop are left alone — a mark
|
||||
# there may have been produced by another command's replacement.
|
||||
segment_text = re.sub(
|
||||
r"(\b(?:" + self._text_cmd_alternation + r")\b)[,.!?;:…]+$",
|
||||
r"\1",
|
||||
segment_text.strip(),
|
||||
flags=re.IGNORECASE,
|
||||
)
|
||||
processed_text = segment_text.strip()
|
||||
|
||||
# Handle action commands
|
||||
@@ -304,14 +325,14 @@ class CommandProcessor:
|
||||
if replacement in _ATTACH_LEFT_REPLACEMENTS:
|
||||
# Punctuation: drop horizontal space before the spoken phrase
|
||||
processed_text = re.sub(
|
||||
_ATTACH_LEFT_WS + cmd_pattern + _ATTACH_LEFT_WS,
|
||||
_ATTACH_LEFT_WS + cmd_pattern + _TRAILING_PUNCTUATION + _ATTACH_LEFT_WS,
|
||||
replacement,
|
||||
processed_text,
|
||||
flags=re.IGNORECASE,
|
||||
)
|
||||
else:
|
||||
processed_text = re.sub(
|
||||
cmd_pattern,
|
||||
cmd_pattern + _TRAILING_PUNCTUATION,
|
||||
replacement,
|
||||
processed_text,
|
||||
flags=re.IGNORECASE,
|
||||
|
||||
@@ -1850,8 +1850,8 @@ class TextInjector:
|
||||
max_retries = 2
|
||||
logger.debug(f"Starting xdotool injection with {max_retries} max retries")
|
||||
|
||||
# Retries resume at the first untyped chunk: restarting from the
|
||||
# top would duplicate the chunks a failed attempt already sent.
|
||||
# Retries resume at the first untyped position: restarting from
|
||||
# the top would duplicate what a failed attempt already sent.
|
||||
typed = 0
|
||||
for retry in range(max_retries + 1):
|
||||
try:
|
||||
@@ -1868,23 +1868,58 @@ class TextInjector:
|
||||
chunk = text[i : i + chunk_size]
|
||||
chunk_num = (i // chunk_size) + 1
|
||||
|
||||
# First try with clearmodifiers. ``--`` ends option
|
||||
# parsing: a chunk that starts with '-' (a word
|
||||
# hyphenated across the chunk boundary) is typed
|
||||
# literally instead of read as a flag and rejected
|
||||
# (#921).
|
||||
cmd = ["xdotool", "type", "--clearmodifiers", "--", chunk]
|
||||
logger.debug(f"Injecting chunk {chunk_num}/{total_chunks}: '{chunk}'")
|
||||
|
||||
subprocess.run(
|
||||
cmd,
|
||||
env=host_env(env),
|
||||
check=True,
|
||||
stderr=subprocess.PIPE,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
typed = min(i + chunk_size, len(text))
|
||||
# ``xdotool type`` maps '\n' to the Linefeed keysym
|
||||
# (0xFF0A), which GTK targets silently drop — only a
|
||||
# real Return keypress inserts the newline (#955).
|
||||
# Split the chunk on '\n', typing each segment and
|
||||
# pressing Return between them. The '\t' case is
|
||||
# untouched: Tab is a working keysym in type strings.
|
||||
segments = chunk.split("\n")
|
||||
for seg_index, segment in enumerate(segments):
|
||||
if self._abort_injections.is_set():
|
||||
raise _InjectionAborted
|
||||
if segment:
|
||||
# ``--`` ends option parsing: a segment that
|
||||
# starts with '-' (a word hyphenated across
|
||||
# the chunk boundary) is typed literally
|
||||
# instead of read as a flag and rejected
|
||||
# (#921).
|
||||
cmd = [
|
||||
"xdotool",
|
||||
"type",
|
||||
"--clearmodifiers",
|
||||
"--",
|
||||
segment,
|
||||
]
|
||||
logger.debug(
|
||||
f"Injecting chunk {chunk_num}/{total_chunks}: '{segment}'"
|
||||
)
|
||||
subprocess.run(
|
||||
cmd,
|
||||
env=host_env(env),
|
||||
check=True,
|
||||
stderr=subprocess.PIPE,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
# Advance past what just went out so a retry
|
||||
# resumes mid-chunk instead of re-typing it.
|
||||
typed += len(segment)
|
||||
if seg_index < len(segments) - 1:
|
||||
subprocess.run(
|
||||
[
|
||||
"xdotool",
|
||||
"key",
|
||||
"--clearmodifiers",
|
||||
"Return",
|
||||
],
|
||||
env=host_env(env),
|
||||
check=True,
|
||||
stderr=subprocess.PIPE,
|
||||
text=True,
|
||||
timeout=5,
|
||||
)
|
||||
typed += 1 # the '\n' the Return produced
|
||||
|
||||
# Add a larger delay between chunks
|
||||
if i + chunk_size < len(text):
|
||||
|
||||
@@ -592,3 +592,36 @@ class TestStandaloneCommandPunctuation(unittest.TestCase):
|
||||
result, actions = self.processor.process_text("...")
|
||||
self.assertEqual(result, "...")
|
||||
self.assertEqual(actions, [])
|
||||
|
||||
def test_mid_utterance_trailing_punctuation_is_consumed(self) -> None:
|
||||
"""Punctuation right after a mid-utterance command does not type (#955)."""
|
||||
cases = [
|
||||
(
|
||||
"Testing 1, 2, 3. New paragraph. Does this work now?",
|
||||
"Testing 1, 2, 3. \n\n Does this work now?",
|
||||
),
|
||||
(
|
||||
"Testing 123, new paragraph, does this work now?",
|
||||
"Testing 123, \n\n does this work now?",
|
||||
),
|
||||
("line one, new line, line two", "line one, \n line two"),
|
||||
("hello new line.", "hello \n"),
|
||||
("hello new line. ", "hello \n"),
|
||||
]
|
||||
for input_text, expected in cases:
|
||||
with self.subTest(input_text=input_text):
|
||||
result, actions = self.processor.process_text(input_text)
|
||||
self.assertEqual(result, expected)
|
||||
self.assertEqual(actions, [])
|
||||
|
||||
def test_trailing_punct_eaten_only_before_space_or_end(self) -> None:
|
||||
"""A dot glued to following text is not sentence punctuation."""
|
||||
result, actions = self.processor.process_text("use new paragraph.done")
|
||||
self.assertEqual(result, "use \n\n.done")
|
||||
self.assertEqual(actions, [])
|
||||
|
||||
def test_command_produced_punctuation_is_not_eaten(self) -> None:
|
||||
"""A mark from an earlier command replacement survives (#955)."""
|
||||
result, actions = self.processor.process_text("hello new line exclamation mark")
|
||||
self.assertEqual(result, "hello \n!")
|
||||
self.assertEqual(actions, [])
|
||||
|
||||
@@ -1139,6 +1139,89 @@ class TestInjectText(unittest.TestCase):
|
||||
mock_xdotool.assert_not_called()
|
||||
self.assertEqual(obj.last_typed_count, 0)
|
||||
|
||||
@staticmethod
|
||||
def _xdotool_calls(run_mock: MagicMock) -> list:
|
||||
"""Return the xdotool type/key argv sequences run_mock received."""
|
||||
return [
|
||||
c.args[0]
|
||||
for c in run_mock.call_args_list
|
||||
if c.args and c.args[0][0] == "xdotool" and c.args[0][1] in ("type", "key")
|
||||
]
|
||||
|
||||
def test_xdotool_newline_sent_as_return_key(self) -> None:
|
||||
"""'\n' is `xdotool key Return`, never a Linefeed inside `type` (#955)."""
|
||||
from vocalinux.text_injection.text_injector import DesktopEnvironment
|
||||
|
||||
obj = _make_injector(DesktopEnvironment.X11)
|
||||
with patch("subprocess.run") as mock_run:
|
||||
self.assertTrue(obj.inject_text("\n"))
|
||||
|
||||
self.assertEqual(
|
||||
self._xdotool_calls(mock_run),
|
||||
[["xdotool", "key", "--clearmodifiers", "Return"]],
|
||||
)
|
||||
|
||||
def test_xdotool_newlines_split_chunk_into_type_and_return(self) -> None:
|
||||
"""A mid-utterance '\n\n' splits the chunk: type, Return, Return (#955)."""
|
||||
from vocalinux.text_injection.text_injector import DesktopEnvironment
|
||||
|
||||
obj = _make_injector(DesktopEnvironment.X11)
|
||||
with patch("subprocess.run") as mock_run:
|
||||
self.assertTrue(obj.inject_text("Testing 123, \n\n, works?"))
|
||||
|
||||
self.assertEqual(
|
||||
self._xdotool_calls(mock_run),
|
||||
[
|
||||
["xdotool", "type", "--clearmodifiers", "--", "Testing 123, "],
|
||||
["xdotool", "key", "--clearmodifiers", "Return"],
|
||||
["xdotool", "key", "--clearmodifiers", "Return"],
|
||||
["xdotool", "type", "--clearmodifiers", "--", ", wor"],
|
||||
["xdotool", "type", "--clearmodifiers", "--", "ks?"],
|
||||
],
|
||||
)
|
||||
|
||||
def test_xdotool_paragraph_break_presses_return_twice(self) -> None:
|
||||
"""'\n\n' from "new paragraph" is two Return presses, no type call (#955)."""
|
||||
from vocalinux.text_injection.text_injector import DesktopEnvironment
|
||||
|
||||
obj = _make_injector(DesktopEnvironment.X11)
|
||||
with patch("subprocess.run") as mock_run:
|
||||
self.assertTrue(obj.inject_text("\n\n"))
|
||||
|
||||
self.assertEqual(
|
||||
self._xdotool_calls(mock_run),
|
||||
[
|
||||
["xdotool", "key", "--clearmodifiers", "Return"],
|
||||
["xdotool", "key", "--clearmodifiers", "Return"],
|
||||
],
|
||||
)
|
||||
|
||||
def test_xdotool_retry_resumes_after_delivered_segment(self) -> None:
|
||||
"""A failed Return retries from the '\n', not from the top (#955)."""
|
||||
from vocalinux.text_injection.text_injector import DesktopEnvironment
|
||||
|
||||
obj = _make_injector(DesktopEnvironment.X11)
|
||||
failed_once = {"done": False}
|
||||
|
||||
def flaky_run(cmd: list, **kwargs: object) -> MagicMock:
|
||||
if cmd[:2] == ["xdotool", "key"] and cmd[-1] == "Return" and not failed_once["done"]:
|
||||
failed_once["done"] = True
|
||||
raise subprocess.CalledProcessError(1, cmd)
|
||||
return MagicMock(returncode=0)
|
||||
|
||||
with patch("subprocess.run", side_effect=flaky_run) as mock_run:
|
||||
self.assertTrue(obj.inject_text("abc\ndef"))
|
||||
|
||||
self.assertEqual(
|
||||
self._xdotool_calls(mock_run),
|
||||
[
|
||||
["xdotool", "type", "--clearmodifiers", "--", "abc"],
|
||||
["xdotool", "key", "--clearmodifiers", "Return"],
|
||||
["xdotool", "key", "--clearmodifiers", "Return"],
|
||||
["xdotool", "type", "--clearmodifiers", "--", "def"],
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
class TestLogWindowInfo(unittest.TestCase):
|
||||
def test_log_x11(self):
|
||||
|
||||
Reference in New Issue
Block a user