From de81403412fc9bbdaf9ea9922270253ff108790a Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 3 Aug 2026 18:19:45 -0700 Subject: [PATCH 1/4] Fix: accept yes/no on --encrypt for Classic parity tabcmd Classic's `--encrypt` on `createextracts` takes an explicit yes/no value; tabcmd 2 treated it as a bare boolean flag, silently ignoring any following value. Scripts that pass `--encrypt no` on Classic ended up encrypting on tabcmd 2 because argparse consumed the "no" as a positional or bailed. Make --encrypt accept an optional yes|no|true|false argument (case-insensitive). Bare `--encrypt` still means True; omitted means False; explicit `--encrypt no` means False. This preserves current tabcmd 2 default behavior while matching Classic syntax. --- tabcmd/execution/global_options.py | 23 ++++++++++++- tests/parsers/test_parser_create_extracts.py | 36 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py index 1a24133c..a58f81f1 100644 --- a/tabcmd/execution/global_options.py +++ b/tabcmd/execution/global_options.py @@ -111,11 +111,32 @@ def set_embedded_datasources_options(parser): # used in create extract. listed in delete-extract but makes no sense there +def _parse_yes_no(value: str) -> bool: + # tabcmd Classic accepted "yes"/"no"/"true"/"false" on --encrypt; tabcmd 2 + # kept the flag but silently ignored a following value. Accept the Classic + # forms so scripts port cleanly. + lowered = value.strip().lower() + if lowered in ("yes", "true", "1"): + return True + if lowered in ("no", "false", "0"): + return False + import argparse + raise argparse.ArgumentTypeError( + "Expected yes/no/true/false for --encrypt, got {!r}".format(value) + ) + + def set_encryption_option(parser): parser.add_argument( "--encrypt", dest="encrypt", - action="store_true", # set to true IF user passes in option --encrypt + # Classic parity: --encrypt yes / --encrypt no. Bare --encrypt still + # means True (default tabcmd 2 behavior). Omitted -> False. + nargs="?", + const=True, + default=False, + type=_parse_yes_no, + metavar="yes|no", help=_("createextracts.options.encrypt"), ) return parser diff --git a/tests/parsers/test_parser_create_extracts.py b/tests/parsers/test_parser_create_extracts.py index 94ca7a7e..0486f2d8 100644 --- a/tests/parsers/test_parser_create_extracts.py +++ b/tests/parsers/test_parser_create_extracts.py @@ -106,3 +106,39 @@ def test_create_extract_parser_missing_embedded_datasources(self): ] with self.assertRaises(SystemExit): args = self.parser_under_test.parse_args(mock_args) + + # --encrypt Classic-parity: accepts yes|no|true|false, bare flag still True, omitted False + def _base_args(self): + return [commandname, "--datasource", "ds", "--project", "p", "--parent-project-path", "pp"] + + def test_encrypt_omitted_is_false(self): + args = self.parser_under_test.parse_args(self._base_args()) + assert args.encrypt is False, args + + def test_encrypt_bare_flag_is_true(self): + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt"]) + assert args.encrypt is True, args + + def test_encrypt_yes_is_true(self): + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "yes"]) + assert args.encrypt is True, args + + def test_encrypt_no_is_false(self): + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "no"]) + assert args.encrypt is False, args + + def test_encrypt_true_is_true(self): + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "true"]) + assert args.encrypt is True, args + + def test_encrypt_false_is_false(self): + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "false"]) + assert args.encrypt is False, args + + def test_encrypt_case_insensitive(self): + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "YES"]) + assert args.encrypt is True, args + + def test_encrypt_bad_value_rejected(self): + with self.assertRaises(SystemExit): + self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "maybe"]) From 01c00648e92ad521641a56d51ead179138ef7890 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Fri, 7 Aug 2026 00:00:41 -0700 Subject: [PATCH 2/4] style: apply black to satisfy CI --- tabcmd/execution/global_options.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py index a58f81f1..e9e29009 100644 --- a/tabcmd/execution/global_options.py +++ b/tabcmd/execution/global_options.py @@ -121,9 +121,8 @@ def _parse_yes_no(value: str) -> bool: if lowered in ("no", "false", "0"): return False import argparse - raise argparse.ArgumentTypeError( - "Expected yes/no/true/false for --encrypt, got {!r}".format(value) - ) + + raise argparse.ArgumentTypeError("Expected yes/no/true/false for --encrypt, got {!r}".format(value)) def set_encryption_option(parser): From fb14edca321fe0d049f4268d8d9ab7184b3f381f Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Fri, 18 Sep 2026 13:18:03 -0700 Subject: [PATCH 3/4] Accept Classic's y/n shortcuts on --encrypt in addition to yes/no MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Classic tabcmd's CreateExtracts.java:79-101 accepts exactly `yes|y|no|n` (case-insensitive) on --encrypt and rejects anything else. This PR was dropping the `y`/`n` shortcuts, which breaks any ported Classic script that used them. Keeping Classic's set intact is the standing rule — tabcmd 2 may be more forgiving than Classic, but must not narrow. Add `y` and `n` to the accepted set (both directions). tabcmd 2 now accepts the union `yes|y|no|n|true|false|1|0`, case-insensitive. Rewrite the source comment to cite the Classic source-of-truth path in the monolith rather than the incorrect earlier claim that Classic accepted "yes/no/true/false". Add two tests pinning `y`→True and `n`→False so a future refactor can't quietly drop them again. Co-Authored-By: Claude Opus 4.7 (1M context) --- tabcmd/execution/global_options.py | 17 +++++++++++------ tests/parsers/test_parser_create_extracts.py | 15 ++++++++++++++- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py index e9e29009..08832d2f 100644 --- a/tabcmd/execution/global_options.py +++ b/tabcmd/execution/global_options.py @@ -112,17 +112,22 @@ def set_embedded_datasources_options(parser): # used in create extract. listed in delete-extract but makes no sense there def _parse_yes_no(value: str) -> bool: - # tabcmd Classic accepted "yes"/"no"/"true"/"false" on --encrypt; tabcmd 2 - # kept the flag but silently ignored a following value. Accept the Classic - # forms so scripts port cleanly. + # tabcmd Classic accepts exactly `yes`/`y`/`no`/`n` (case-insensitive) on + # --encrypt — see workgroup/src/silos/tableau-server/applications/ + # app-tabcmd/src/com/tableausoftware/tabcmd/commands/CreateExtracts.java + # lines 79-101 in the monolith. tabcmd 2 keeps Classic's set intact (so + # ported scripts run) and adds `true`/`false`/`1`/`0` as a forgiving + # superset for users who type boolean-shaped values. lowered = value.strip().lower() - if lowered in ("yes", "true", "1"): + if lowered in ("yes", "y", "true", "1"): return True - if lowered in ("no", "false", "0"): + if lowered in ("no", "n", "false", "0"): return False import argparse - raise argparse.ArgumentTypeError("Expected yes/no/true/false for --encrypt, got {!r}".format(value)) + raise argparse.ArgumentTypeError( + "Expected yes/y/no/n/true/false/1/0 for --encrypt, got {!r}".format(value) + ) def set_encryption_option(parser): diff --git a/tests/parsers/test_parser_create_extracts.py b/tests/parsers/test_parser_create_extracts.py index 0486f2d8..ce36b952 100644 --- a/tests/parsers/test_parser_create_extracts.py +++ b/tests/parsers/test_parser_create_extracts.py @@ -107,7 +107,10 @@ def test_create_extract_parser_missing_embedded_datasources(self): with self.assertRaises(SystemExit): args = self.parser_under_test.parse_args(mock_args) - # --encrypt Classic-parity: accepts yes|no|true|false, bare flag still True, omitted False + # --encrypt Classic set is yes|y|no|n (case-insensitive) per + # app-tabcmd/.../CreateExtracts.java:79-101; tabcmd 2 adds + # true|false|1|0 on top as a forgiving superset. Bare flag still True, + # omitted False. def _base_args(self): return [commandname, "--datasource", "ds", "--project", "p", "--parent-project-path", "pp"] @@ -127,6 +130,16 @@ def test_encrypt_no_is_false(self): args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "no"]) assert args.encrypt is False, args + def test_encrypt_y_is_true(self): + # Classic shortcut; ported scripts must keep working. + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "y"]) + assert args.encrypt is True, args + + def test_encrypt_n_is_false(self): + # Classic shortcut; ported scripts must keep working. + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "n"]) + assert args.encrypt is False, args + def test_encrypt_true_is_true(self): args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "true"]) assert args.encrypt is True, args From 0912a7d9d73d75c8d40756a396efeb9e3735a35f Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Sat, 19 Sep 2026 19:29:07 -0700 Subject: [PATCH 4/4] docs: expand --encrypt metavar to advertise full accepted alias set Addresses Copilot review feedback on PR #450: the help metavar showed only "yes|no" while _parse_yes_no accepts yes/y/no/n/true/false/1/0. Updates metavar to reflect the full forgiving superset and adds two tests locking in 1/0 numeric-alias acceptance so the docs contract and the parser stay in sync. --- tabcmd/execution/global_options.py | 2 +- tests/parsers/test_parser_create_extracts.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py index 08832d2f..08ef205f 100644 --- a/tabcmd/execution/global_options.py +++ b/tabcmd/execution/global_options.py @@ -140,7 +140,7 @@ def set_encryption_option(parser): const=True, default=False, type=_parse_yes_no, - metavar="yes|no", + metavar="yes|y|no|n|true|false|1|0", help=_("createextracts.options.encrypt"), ) return parser diff --git a/tests/parsers/test_parser_create_extracts.py b/tests/parsers/test_parser_create_extracts.py index ce36b952..505cee14 100644 --- a/tests/parsers/test_parser_create_extracts.py +++ b/tests/parsers/test_parser_create_extracts.py @@ -148,6 +148,16 @@ def test_encrypt_false_is_false(self): args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "false"]) assert args.encrypt is False, args + def test_encrypt_1_is_true(self): + # Numeric alias in tabcmd 2's forgiving superset. + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "1"]) + assert args.encrypt is True, args + + def test_encrypt_0_is_false(self): + # Numeric alias in tabcmd 2's forgiving superset. + args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "0"]) + assert args.encrypt is False, args + def test_encrypt_case_insensitive(self): args = self.parser_under_test.parse_args(self._base_args() + ["--encrypt", "YES"]) assert args.encrypt is True, args