From d3ba4ee743dde2fe82da52df915408cdd0f62687 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 16 Oct 2026 19:05:34 -0700 Subject: [PATCH] fix(injection): send newlines as Return on the xdotool backend (#962) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../speech_recognition/command_processor.py | 27 +++++- src/vocalinux/text_injection/text_injector.py | 73 +++++++++++----- tests/test_command_processor.py | 33 ++++++++ tests/test_text_injector_ext.py | 83 +++++++++++++++++++ 4 files changed, 194 insertions(+), 22 deletions(-) diff --git a/src/vocalinux/speech_recognition/command_processor.py b/src/vocalinux/speech_recognition/command_processor.py index 8c07ce0..b43a517 100644 --- a/src/vocalinux/speech_recognition/command_processor.py +++ b/src/vocalinux/speech_recognition/command_processor.py @@ -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, diff --git a/src/vocalinux/text_injection/text_injector.py b/src/vocalinux/text_injection/text_injector.py index a64ae70..2da8e6a 100644 --- a/src/vocalinux/text_injection/text_injector.py +++ b/src/vocalinux/text_injection/text_injector.py @@ -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): diff --git a/tests/test_command_processor.py b/tests/test_command_processor.py index 47fa6c9..6048e5d 100644 --- a/tests/test_command_processor.py +++ b/tests/test_command_processor.py @@ -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, []) diff --git a/tests/test_text_injector_ext.py b/tests/test_text_injector_ext.py index 988ea76..8fed5ad 100644 --- a/tests/test_text_injector_ext.py +++ b/tests/test_text_injector_ext.py @@ -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):