fix: do not append empty/comment-only input to history (#4822)

* add test for importing empty .xsh file

* test: empty lines do not get appended to history

prompt-toolkit needs its own test outside of test_base_shell.py because it uses a custom _push() method

* fix: do not append empty/comment-only input to history

Adds a compile_empty_tree argument to Execer.compile()
By default, the argument is `True`, and `compile()` returns a compiled `pass` statement for comment-only input.
When the argument is `False`, `compile()` returns `None` for comment-only input.

The base shell and prompt-toolkit shell use `compile_empty_tree = False` so that they get `None` as the compiled code and don't append the command to the history.

* add news

* fix tests

news/fix-ptk-force-quit.rst

@@ -0,0 +1,24 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* Empty/comment-only commands no longer get added to the history
+* On prompt-toolkit, when there is a job like `sleep 500 &` running in the background, pressing Ctrl+D twice to force quit now works properly
+
+**Security:**
+
+* <news item>

tests/test_base_shell.py

@@ -1,6 +1,8 @@
 """(A down payment on) Testing for ``xonsh.base_shell.BaseShell`` and associated classes"""
 import os
 
+import pytest
+
 from xonsh.base_shell import BaseShell
 from xonsh.shell import transform_command
 
@@ -36,3 +38,28 @@ def test_transform(xession):
     assert transform_command("spam") == "egg"
     assert transform_command("egg") == "egg"
     assert transform_command("foo") == "foo"
+
+
+@pytest.mark.parametrize(
+    "cmd,exp_append_history",
+    [
+        ("", False),
+        ("# a comment", False),
+        ("print('yes')", True),
+    ],
+)
+def test_default_append_history(cmd, exp_append_history, xonsh_session, monkeypatch):
+    """Test that running an empty line or a comment does not append to history"""
+    append_history_calls = []
+
+    def mock_append_history(**info):
+        append_history_calls.append(info)
+
+    monkeypatch.setattr(
+        xonsh_session.shell.shell, "_append_history", mock_append_history
+    )
+    xonsh_session.shell.default(cmd)
+    if exp_append_history:
+        assert len(append_history_calls) == 1
+    else:
+        assert len(append_history_calls) == 0

tests/test_imphooks.py

@@ -20,6 +20,12 @@ def test_import():
     assert "hello mom jawaka\n" == sample.x
 
 
+def test_import_empty():
+    from xpack import empty_xsh
+
+    assert empty_xsh
+
+
 def test_absolute_import():
     from xpack import sample
 

tests/test_ptk_shell.py

@@ -121,3 +121,28 @@ def test_ptk_prompt(line, exp, ptk_shell, capsys):
     out = screen.display[0].strip()
 
     assert out.strip() == (exp or line)
+
+
+@pytest.mark.parametrize(
+    "cmd,exp_append_history",
+    [
+        ("", False),
+        ("# a comment", False),
+        ("print('yes')", True),
+    ],
+)
+def test_ptk_default_append_history(cmd, exp_append_history, ptk_shell, monkeypatch):
+    """Test that running an empty line or a comment does not append to history.
+    This test is necessary because the prompt-toolkit shell uses a custom _push() method that is different from the base shell's push() method."""
+    inp, out, shell = ptk_shell
+    append_history_calls = []
+
+    def mock_append_history(**info):
+        append_history_calls.append(info)
+
+    monkeypatch.setattr(shell, "_append_history", mock_append_history)
+    shell.default(cmd)
+    if exp_append_history:
+        assert len(append_history_calls) == 1
+    else:
+        assert len(append_history_calls) == 0

xonsh/base_shell.py

@@ -511,7 +511,12 @@ class BaseShell:
             return src, None
         try:
             code = self.execer.compile(
-                src, mode="single", glbs=self.ctx, locs=None, filename="<stdin>"
+                src,
+                mode="single",
+                glbs=self.ctx,
+                locs=None,
+                filename="<stdin>",
+                compile_empty_tree=False,
             )
             if _cache:
                 update_cache(code, cachefname)

xonsh/execer.py

@@ -112,6 +112,7 @@ class Execer:
         stacklevel=2,
         filename=None,
         transform=True,
+        compile_empty_tree=True,
     ):
         """Compiles xonsh code into a Python code object, which may then
         be execed or evaled.
@@ -128,7 +129,9 @@ class Execer:
         ctx = set(dir(builtins)) | set(glbs.keys()) | set(locs.keys())
         tree = self.parse(input, ctx, mode=mode, filename=filename, transform=transform)
         if tree is None:
-            return compile("pass", filename, mode)  # handles comment only input
+            return (
+                compile("pass", filename, mode) if compile_empty_tree else None
+            )  # handles comment only input
         try:
             code = compile(tree, filename, mode)
         except SyntaxError as e:

xonsh/ptk_shell/shell.py

@@ -188,6 +188,7 @@ class PromptToolkitShell(BaseShell):
             winutils.enable_virtual_terminal_processing()
         self._first_prompt = True
         self.history = ThreadedHistory(PromptToolkitHistory())
+        self.push = self._push
 
         ptk_args.setdefault("history", self.history)
         if not XSH.env.get("XONSH_COPY_ON_DELETE", False):
@@ -380,7 +381,9 @@ class PromptToolkitShell(BaseShell):
         src = "".join(self.buffer)
         src = transform_command(src)
         try:
-            code = self.execer.compile(src, mode="single", glbs=self.ctx, locs=None)
+            code = self.execer.compile(
+                src, mode="single", glbs=self.ctx, locs=None, compile_empty_tree=False
+            )
             self.reset_buffer()
         except Exception:  # pylint: disable=broad-except
             self.reset_buffer()
@@ -393,7 +396,6 @@ class PromptToolkitShell(BaseShell):
         if intro:
             print(intro)
         auto_suggest = AutoSuggestFromHistory()
-        self.push = self._push
         while not XSH.exit:
             try:
                 line = self.singleline(auto_suggest=auto_suggest)