From c6590c64f690218ec6b2c6158ec3d08f6d3b4ccd Mon Sep 17 00:00:00 2001 From: Sahil Sunny Date: Thu, 9 Jul 2026 13:39:42 +0530 Subject: [PATCH] Add warning for session defaults Session defaults persists until manually removed from the session. So when using commands that are not compatible with the params, the system simply ignores the params. Instead of it, this fix adds a warning that the param was skipped due to non-compatibility. So it will apply when running valid commands and warn when running invalid commands but it just continues to execute after the warning. --- src/scrapingbee_cli/interactive.py | 21 +++++++-- tests/unit/test_repl_pty.py | 63 +++++++++++++++++++++++++ tests/unit/test_scrollback_selection.py | 37 +++++++++++++++ 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/scrapingbee_cli/interactive.py b/src/scrapingbee_cli/interactive.py index 339ef03..6741f09 100644 --- a/src/scrapingbee_cli/interactive.py +++ b/src/scrapingbee_cli/interactive.py @@ -863,7 +863,7 @@ def __init__(self) -> None: def apply_settings_to_args( self, args: list[str], accepted: set[str] | None = None - ) -> list[str]: + ) -> tuple[list[str], list[str]]: """Append session defaults to ``args`` for any flag that: - is not already present on the command line, AND - is accepted by the target command (when ``accepted`` is given). @@ -871,19 +871,24 @@ def apply_settings_to_args( Without the ``accepted`` filter, session defaults would leak into commands that don't take them (e.g. ``--json-response`` into ``usage``), causing "No such option" errors. + + Returns ``(args, skipped)`` where ``skipped`` lists setting keys that + were not applied because the target command does not accept them. """ if not self.settings: - return args + return args, [] present = {a for a in args if a.startswith("--")} out = list(args) + skipped: list[str] = [] for key, value in self.settings.items(): flag = f"--{key}" if flag in present: continue if accepted is not None and flag not in accepted: + skipped.append(key) continue out.extend([flag, value]) - return out + return out, skipped def refresh_credits_from_cache(self) -> None: """Populate live fields from the on-disk usage cache. @@ -4304,7 +4309,15 @@ def _execute(line: str) -> bool: # Only inject session defaults that the target command actually # accepts; otherwise ``:set --json-response true`` would also # apply to ``usage``, which rejects it as an unknown option. - args = state.apply_settings_to_args(args, accepted=set(command_flags.get(cmd_name, []))) + args, skipped_settings = state.apply_settings_to_args( + args, accepted=set(command_flags.get(cmd_name, [])) + ) + if skipped_settings: + flags = ", ".join(f"--{k}" for k in skipped_settings) + err_console.print( + f" [{BEE_DIM}]note:[/] session default(s) {flags} not applied to " + f"[bold {BEE_YELLOW}]{cmd_name}[/] (unsupported by this command)" + ) # Let users type ``--verbose true|false`` (etc.) in the REPL # too — same normalisation as the CLI ``main()`` entry. try: diff --git a/tests/unit/test_repl_pty.py b/tests/unit/test_repl_pty.py index 722b04d..0176fe8 100644 --- a/tests/unit/test_repl_pty.py +++ b/tests/unit/test_repl_pty.py @@ -251,3 +251,66 @@ def test_classic_mouse_shift_tab_toggles_mode(tmp_path): ) finally: child.close(force=True) + + +def _has_session_default_skip_warning(screen, command: str, setting: str) -> bool: + t = _text(screen) + return ( + "not applied to" in t + and command in t + and setting in t + and "unsupported by this command" in t + ) + + +@needs_cli +def test_session_default_skip_warning_on_screen(tmp_path): + """Scrape-only session defaults warn on screen when a command ignores them.""" + child, screen, stream = _spawn(tmp_path) + try: + assert _pump_until(child, screen, stream, lambda s: "❯" in _text(s)), "no prompt" + child.send(":set premium-proxy=true\r") + assert _pump_until( + child, + screen, + stream, + lambda s: "premium-proxy" in _text(s) and "true" in _text(s), + ), ":set did not apply premium-proxy=true" + child.send("google --help\r") + assert _pump_until( + child, + screen, + stream, + lambda s: _has_session_default_skip_warning(s, "google", "premium-proxy"), + timeout=20.0, + ), "skip warning for premium-proxy on google not shown" + finally: + child.close(force=True) + + +@needs_cli +def test_session_default_no_skip_warning_for_supported_command(tmp_path): + """Session defaults supported by the target command must not emit a skip warning.""" + child, screen, stream = _spawn(tmp_path) + try: + assert _pump_until(child, screen, stream, lambda s: "❯" in _text(s)), "no prompt" + child.send(":set premium-proxy=true\r") + assert _pump_until( + child, + screen, + stream, + lambda s: "premium-proxy" in _text(s) and "true" in _text(s), + ), ":set did not apply premium-proxy=true" + child.send("scrape --help\r") + assert _pump_until( + child, + screen, + stream, + lambda s: "✓" in _text(s) and "--output-file" in _text(s), + timeout=20.0, + ), "scrape --help did not complete" + assert not _has_session_default_skip_warning(screen, "scrape", "premium-proxy"), ( + "skip warning shown for premium-proxy on scrape" + ) + finally: + child.close(force=True) diff --git a/tests/unit/test_scrollback_selection.py b/tests/unit/test_scrollback_selection.py index fa505a8..f794278 100644 --- a/tests/unit/test_scrollback_selection.py +++ b/tests/unit/test_scrollback_selection.py @@ -159,3 +159,40 @@ def test_no_tool_returns_false(self, monkeypatch): monkeypatch.setattr(shutil, "which", lambda name: None) assert _copy_to_clipboard("hello") is False + + +class TestSessionDefaults: + def test_apply_settings_filters_unsupported_flags(self): + from scrapingbee_cli.interactive import SessionState + + state = SessionState() + state.settings["premium-proxy"] = "true" + state.settings["verbose"] = "true" + + args, skipped = state.apply_settings_to_args( + ["test query"], + accepted={"--verbose", "--country-code"}, + ) + assert args == ["test query", "--verbose", "true"] + assert skipped == ["premium-proxy"] + + def test_apply_settings_no_skip_when_flag_on_command_line(self): + from scrapingbee_cli.interactive import SessionState + + state = SessionState() + state.settings["premium-proxy"] = "true" + + args, skipped = state.apply_settings_to_args( + ["https://example.com", "--premium-proxy", "false"], + accepted={"--premium-proxy"}, + ) + assert args == ["https://example.com", "--premium-proxy", "false"] + assert skipped == [] + + def test_apply_settings_empty_when_no_settings(self): + from scrapingbee_cli.interactive import SessionState + + state = SessionState() + args, skipped = state.apply_settings_to_args(["query"], accepted={"--verbose"}) + assert args == ["query"] + assert skipped == []