🩹 fix(interactive-settings): improve input cancellation handling && update reset tests && sort filtered diff files

Author: Jannchie

tests/unit/test_interactive_settings.py

@@ -149,16 +149,16 @@ def test_configure_global_settings_cancel_api_key(self, mock_text, mock_load):
 
         mock_text.assert_called_once()
 
-    @patch("tgit.interactive_settings.load_global_settings")
-    @patch("tgit.interactive_settings.questionary.text")
-    @patch("tgit.interactive_settings.questionary.confirm")
+    @patch("pathlib.Path.write_text")
+    @patch("tgit.interactive_settings.json.dumps")
+    @patch("tgit.interactive_settings.json.loads")
     @patch("tgit.interactive_settings.Path.home")
-    @patch("tgit.interactive_settings.yaml.safe_load")
-    @patch("tgit.interactive_settings.yaml.safe_dump")
-    @patch("builtins.open", new_callable=mock_open)
-    def test_configure_global_settings_complete(self, mock_file, mock_dump, mock_load_yaml, mock_home, mock_confirm, mock_text):
+    @patch("tgit.interactive_settings.questionary.confirm")
+    @patch("tgit.interactive_settings.questionary.text")
+    @patch("tgit.interactive_settings.load_global_settings")
+    def test_configure_global_settings_complete(self, mock_load_global_settings, mock_text, mock_confirm, mock_home, mock_loads, mock_dumps, mock_write_text):
         """Test _configure_global_settings complete flow."""
-        mock_load_yaml.return_value = {}
+        mock_loads.return_value = {}
         mock_home.return_value = Path("/home/user")
 
         # Mock all questionary inputs
@@ -180,17 +180,19 @@ def test_configure_global_settings_complete(self, mock_file, mock_dump, mock_loa
         # Verify all inputs were called
         assert mock_text.call_count == 3
         assert mock_confirm.call_count == 4
-        mock_dump.assert_called_once()
+        mock_write_text.assert_called_once()
 
 
 class TestConfigureWorkspaceSettings:
     """Test _configure_workspace_settings function."""
 
     @patch("tgit.interactive_settings.load_workspace_settings")
+    @patch("tgit.interactive_settings.questionary.confirm")
     @patch("tgit.interactive_settings.questionary.text")
-    def test_configure_workspace_settings_cancel(self, mock_text, mock_load):
+    def test_configure_workspace_settings_cancel(self, mock_text, mock_confirm, mock_load):
         """Test _configure_workspace_settings with cancel."""
         mock_load.return_value = {}
+        mock_confirm.return_value.ask.return_value = True
         mock_text.return_value.ask.return_value = None
 
         _configure_workspace_settings()
@@ -209,64 +211,42 @@ def test_reset_settings_cancel(self, mock_print, mock_select):
 
         _reset_settings()
 
-        mock_print.assert_any_call("\n[bold red]Reset Settings[/bold red]")
+        mock_print.assert_not_called()
 
     @patch("tgit.interactive_settings.questionary.select")
     @patch("tgit.interactive_settings.questionary.confirm")
     @patch("tgit.interactive_settings.Path.home")
     @patch("tgit.interactive_settings.print")
     def test_reset_settings_global_confirmed(self, mock_print, mock_home, mock_confirm, mock_select):
         """Test _reset_settings for global settings with confirmation."""
-        mock_select.return_value.ask.return_value = "Reset global settings"
+        mock_select.return_value.ask.return_value = "global"
         mock_confirm.return_value.ask.return_value = True
         mock_home.return_value = Path("/home/user")
 
-        with patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.unlink") as mock_unlink:
-            _reset_settings()
-            
-            mock_confirm.assert_called_once()
-            mock_unlink.assert_called_once()
-
-    @patch("tgit.interactive_settings.questionary.select")
-    @patch("tgit.interactive_settings.questionary.confirm")
-    @patch("tgit.interactive_settings.print")
-    def test_reset_settings_workspace_confirmed(self, mock_print, mock_confirm, mock_select):
-        """Test _reset_settings for workspace settings with confirmation."""
-        mock_select.return_value.ask.return_value = "Reset workspace settings"
-        mock_confirm.return_value.ask.return_value = True
-        
-        with patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.unlink") as mock_unlink:
+        with (
+            patch("pathlib.Path.exists", return_value=True) as mock_exists,
+            patch("pathlib.Path.unlink") as mock_unlink,
+        ):
             _reset_settings()
 
             mock_confirm.assert_called_once()
             mock_unlink.assert_called_once()
+            mock_print.assert_any_call("[green]Global settings reset successfully![/green]")
 
     @patch("tgit.interactive_settings.questionary.select")
     @patch("tgit.interactive_settings.questionary.confirm")
     @patch("tgit.interactive_settings.Path.home")
     @patch("tgit.interactive_settings.print")
     def test_reset_settings_global_cancelled(self, mock_print, mock_home, mock_confirm, mock_select):
         """Test _reset_settings for global settings cancelled."""
-        mock_select.return_value.ask.return_value = "Reset global settings"
+        mock_select.return_value.ask.return_value = "global"
         mock_confirm.return_value.ask.return_value = False
         mock_home.return_value = Path("/home/user")
 
-        with patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.unlink") as mock_unlink:
-            _reset_settings()
-            
-            mock_confirm.assert_called_once()
-            mock_unlink.assert_not_called()
-            mock_print.assert_any_call("[yellow]Reset cancelled.[/yellow]")
-
-    @patch("tgit.interactive_settings.questionary.select")
-    @patch("tgit.interactive_settings.questionary.confirm")
-    @patch("tgit.interactive_settings.print")
-    def test_reset_settings_workspace_cancelled(self, mock_print, mock_confirm, mock_select):
-        """Test _reset_settings for workspace settings cancelled."""
-        mock_select.return_value.ask.return_value = "Reset workspace settings"
-        mock_confirm.return_value.ask.return_value = False
-        
-        with patch("pathlib.Path.exists", return_value=True), patch("pathlib.Path.unlink") as mock_unlink:
+        with (
+            patch("pathlib.Path.exists", return_value=True) as mock_exists,
+            patch("pathlib.Path.unlink") as mock_unlink,
+        ):
             _reset_settings()
 
             mock_confirm.assert_called_once()
@@ -279,11 +259,12 @@ def test_reset_settings_workspace_cancelled(self, mock_print, mock_confirm, mock
     @patch("tgit.interactive_settings.print")
     def test_reset_settings_global_file_not_exists(self, mock_print, mock_home, mock_confirm, mock_select):
         """Test _reset_settings for global settings when file doesn't exist."""
-        mock_select.return_value.ask.return_value = "Reset global settings"
+        mock_select.return_value.ask.return_value = "global"
         mock_confirm.return_value.ask.return_value = True
         mock_home.return_value = Path("/home/user")
 
-        with patch("pathlib.Path.exists", return_value=False), patch("pathlib.Path.unlink") as mock_unlink:
+        with patch("pathlib.Path.exists", return_value=False) as mock_exists, \
+             patch("pathlib.Path.unlink") as mock_unlink:
             _reset_settings()
 
             mock_confirm.assert_called_once()
@@ -295,12 +276,34 @@ def test_reset_settings_global_file_not_exists(self, mock_print, mock_home, mock
     @patch("tgit.interactive_settings.print")
     def test_reset_settings_workspace_file_not_exists(self, mock_print, mock_confirm, mock_select):
         """Test _reset_settings for workspace settings when file doesn't exist."""
-        mock_select.return_value.ask.return_value = "Reset workspace settings"
+        mock_select.return_value.ask.return_value = "workspace"
         mock_confirm.return_value.ask.return_value = True
 
-        with patch("pathlib.Path.exists", return_value=False), patch("pathlib.Path.unlink") as mock_unlink:
+        with patch("pathlib.Path.exists", return_value=False) as mock_exists, \
+             patch("pathlib.Path.unlink") as mock_unlink:
             _reset_settings()
 
             mock_confirm.assert_called_once()
             mock_unlink.assert_not_called()
             mock_print.assert_any_call("[yellow]Workspace settings file does not exist.[/yellow]")
+
+    @patch("tgit.interactive_settings.questionary.select")
+    @patch("tgit.interactive_settings.questionary.confirm")
+    @patch("tgit.interactive_settings.Path.home")
+    @patch("tgit.interactive_settings.print")
+    def test_reset_settings_both(self, mock_print, mock_home, mock_confirm, mock_select):
+        """Test _reset_settings for both global and workspace settings."""
+        mock_select.return_value.ask.return_value = "both"
+        mock_confirm.return_value.ask.return_value = True
+        mock_home.return_value = Path("/home/user")
+        
+        with (
+            patch("pathlib.Path.exists", return_value=True) as mock_exists,
+            patch("pathlib.Path.unlink") as mock_unlink,
+        ):
+            _reset_settings()
+            
+            mock_confirm.assert_called_once()
+            assert mock_unlink.call_count == 2
+            mock_print.assert_any_call("[green]Global settings reset successfully![/green]")
+            mock_print.assert_any_call("[green]Workspace settings reset successfully![/green]")

tgit/commit.py

@@ -122,7 +122,7 @@ def get_filtered_diff_files(repo: git.Repo) -> tuple[list[str], list[str]]:
     lock_files: list[str] = []
 
     # 过滤文件
-    for filename in all_changed_files:
+    for filename in sorted(all_changed_files):
         if filename.endswith(".lock"):
             lock_files.append(filename)
             continue

tgit/interactive_settings.py

@@ -159,45 +159,48 @@ def _configure_workspace_settings() -> None:
     ).ask():
         return
 
-    # Collect all inputs without early returns
-    api_key = (
-        questionary.text(
-            "OpenAI API Key (workspace override)",
-            default=current_settings.get("apiKey", ""),
-        ).ask()
-        or ""
-    )
+    # Collect all inputs with early returns on cancel
+    api_key = questionary.text(
+        "OpenAI API Key (workspace override)",
+        default=current_settings.get("apiKey", ""),
+    ).ask()
+    if api_key is None:
+        return
 
-    api_url = (
-        questionary.text(
-            "API URL (workspace override)",
-            default=current_settings.get("apiUrl", ""),
-        ).ask()
-        or ""
-    )
+    api_url = questionary.text(
+        "API URL (workspace override)",
+        default=current_settings.get("apiUrl", ""),
+    ).ask()
+    if api_url is None:
+        return
 
-    model = (
-        questionary.text(
-            "Model name (workspace override)",
-            default=current_settings.get("model", ""),
-        ).ask()
-        or ""
-    )
+    model = questionary.text(
+        "Model name (workspace override)",
+        default=current_settings.get("model", ""),
+    ).ask()
+    if model is None:
+        return
 
     show_command = questionary.confirm(
         "Show git commands before execution",
         default=current_settings.get("show_command", True),
     ).ask()
+    if show_command is None:
+        return
 
     skip_confirm = questionary.confirm(
         "Skip confirmation prompts",
         default=current_settings.get("skip_confirm", False),
     ).ask()
+    if skip_confirm is None:
+        return
 
     commit_emoji = questionary.confirm(
         "Use emoji in commit messages",
         default=current_settings.get("commit", {}).get("emoji", False),
     ).ask()
+    if commit_emoji is None:
+        return
 
     # Build settings dictionary
     new_settings = {