From 1b318f9a5dfbf20d0c500687a0982153ff4417d6 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Wed, 29 Jul 2026 15:54:06 -0700 Subject: [PATCH 1/3] feat: allow repeated --filter flags for exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously `--filter` accepted only one value, and multi-pair filtering required joining pairs with '&' (e.g. `--filter "A=1&B=2"`). That parsing scheme conflicts with values that legitimately contain '&' (e.g. `Product Name=AT&T ...`). Make `--filter` repeatable via argparse action="append". Each flag now carries exactly one COLUMN=VALUE pair — no '&' splitting — so literal '&' in a value passes through untouched. Back-compat: when only one --filter flag is present, the value is still split on '&' so existing scripts keep working. Users who need '&' in a value should switch to the repeated-flag form. Adds parser tests for the new syntax and unit tests for apply_filters_from_args covering both forms. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../export_command.py | 19 +++++++++---- .../locales/en/tabcmd_messages_en.properties | 2 +- .../test_datasources_and_workbooks_command.py | 27 +++++++++++++++++++ tests/parsers/test_parser_export.py | 14 ++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 71e902b4..607c3682 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -68,6 +68,7 @@ def define_args(export_parser): group.add_argument( "--filter", metavar="COLUMN=VALUE", + action="append", help=_("tabcmd.export.help.filter"), ) group.add_argument( @@ -143,11 +144,19 @@ def run_command(cls, args): @staticmethod def apply_filters_from_args(request_options: RequestOptionsType, args, logger=None) -> None: - if args.filter: - logger.debug("filter = {}".format(args.filter)) - params = args.filter.split("&") - for value in params: - ExportCommand.apply_filter_value(logger, request_options, value) + if not args.filter: + return + logger.debug("filter = {}".format(args.filter)) + # Back-compat: a single --filter flag historically joined multiple pairs + # with '&' (e.g. `--filter "a=1&b=2"`), so that one case still splits on '&'. + # Repeated --filter flags each carry exactly one pair — no split — so a + # literal '&' or '=' in a value is passed through untouched. + if len(args.filter) == 1: + values = args.filter[0].split("&") + else: + values = args.filter + for value in values: + ExportCommand.apply_filter_value(logger, request_options, value) @staticmethod def download_wb_pdf(server, workbook_item, args, logger): diff --git a/tabcmd/locales/en/tabcmd_messages_en.properties b/tabcmd/locales/en/tabcmd_messages_en.properties index 7eda4bff..d4f7e238 100644 --- a/tabcmd/locales/en/tabcmd_messages_en.properties +++ b/tabcmd/locales/en/tabcmd_messages_en.properties @@ -155,7 +155,7 @@ tabcmd.error.server_connection_failed=Failed to connect to server tabcmd.errors.parent.not.found=Containing project could not be found tabcmd.errors.user_already_exists=A user with the name {0} already exists. Try a different name. tabcmd.export.help.filename=Filename to store the exported data -tabcmd.export.help.filter=Data filter to apply to the view +tabcmd.export.help.filter=Data filter to apply to the view (COLUMN=VALUE). Repeat the flag for multiple filters (--filter "A=1" --filter "B=2"); repeated flags allow '&' inside a value. For back-compat, a single --filter may still join pairs with '&' (--filter "A=1&B=2"). tabcmd.export.help.orientation=Page orientation (landscape or portrait) of the exported PDF tabcmd.export.help.page_size=Set the page size of the exported PDF tabcmd.export.help.url=URL of the workbook or view to export diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 8fa60dde..4afe2497 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock from tabcmd.commands.datasources_and_workbooks.datasources_and_workbooks_command import DatasourcesAndWorkbooks +from tabcmd.commands.datasources_and_workbooks.export_command import ExportCommand import tableauserverclient as tsc import unittest from unittest import mock @@ -189,6 +190,32 @@ def test_apply_csv_options_with_language(self): DatasourcesAndWorkbooks.apply_csv_options(mock_logger, request_options, mock_args) assert request_options.language == "de" + def test_apply_filters_from_args_none(self): + args = argparse.Namespace(filter=None) + request_options = tsc.PDFRequestOptions() + ExportCommand.apply_filters_from_args(request_options, args, mock_logger) + assert request_options.view_filters == [] + + def test_apply_filters_from_args_single(self): + args = argparse.Namespace(filter=["Region=West"]) + request_options = tsc.PDFRequestOptions() + ExportCommand.apply_filters_from_args(request_options, args, mock_logger) + assert request_options.view_filters == [("Region", "West")] + + def test_apply_filters_from_args_repeated(self): + # repeated --filter flags each carry one pair, so '&' in a value is safe. + args = argparse.Namespace(filter=["Region=West", "Product=AT&T"]) + request_options = tsc.PDFRequestOptions() + ExportCommand.apply_filters_from_args(request_options, args, mock_logger) + assert request_options.view_filters == [("Region", "West"), ("Product", "AT&T")] + + def test_apply_filters_from_args_backcompat_ampersand_join(self): + # Old-style single flag joining pairs with '&' still parses. + args = argparse.Namespace(filter=["Region=West&Product=Widget"]) + request_options = tsc.PDFRequestOptions() + ExportCommand.apply_filters_from_args(request_options, args, mock_logger) + assert request_options.view_filters == [("Region", "West"), ("Product", "Widget")] + @mock.patch("tableauserverclient.Server") class MockedServerTests(unittest.TestCase): diff --git a/tests/parsers/test_parser_export.py b/tests/parsers/test_parser_export.py index 585f500c..5ac8463f 100644 --- a/tests/parsers/test_parser_export.py +++ b/tests/parsers/test_parser_export.py @@ -21,3 +21,17 @@ def test_export_parser_missing_all_args(self): mock_args = [commandname] with self.assertRaises(SystemExit): args = self.parser_under_test.parse_args(mock_args) + + def test_export_parser_single_filter(self): + args = self.parser_under_test.parse_args([commandname, "helloworld", "--pdf", "--filter", "Region=West"]) + assert args.filter == ["Region=West"] + + def test_export_parser_repeated_filter(self): + args = self.parser_under_test.parse_args( + [commandname, "helloworld", "--pdf", "--filter", "Region=West", "--filter", "Product=AT&T"] + ) + assert args.filter == ["Region=West", "Product=AT&T"] + + def test_export_parser_no_filter_defaults_to_none(self): + args = self.parser_under_test.parse_args([commandname, "helloworld", "--pdf"]) + assert args.filter is None From 9adad8f9601a9c297b25a1415d672d7d44c710de Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 10 Sep 2026 12:34:26 -0700 Subject: [PATCH 2/3] Remove dead default=None on logger param All call sites pass logger explicitly, so the default was unreachable. Tightens the type from Optional[Logger] to Logger, drops any fallback branch inside. Co-Authored-By: Claude Opus 4.7 (1M context) --- tabcmd/commands/datasources_and_workbooks/export_command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 607c3682..78e32a61 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -143,7 +143,7 @@ def run_command(cls, args): Errors.exit_with_error(logger, "Error saving to file", exception=e) @staticmethod - def apply_filters_from_args(request_options: RequestOptionsType, args, logger=None) -> None: + def apply_filters_from_args(request_options: RequestOptionsType, args, logger) -> None: if not args.filter: return logger.debug("filter = {}".format(args.filter)) From 571424894fa41f773fe6a33de0660d01f12bb619 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 20 Oct 2026 16:38:37 -0700 Subject: [PATCH 3/3] fix: address Copilot findings on filter parsing for repeated --filter - apply_filter_value now splits on the first '=' only, so values containing '=' (e.g. Notes=x=y) are no longer truncated. - apply_filters_from_args only treats a literal '&' in a single --filter value as a back-compat pair delimiter when every resulting segment actually looks like COLUMN=VALUE; otherwise the '&' is treated as literal (e.g. Company=AT&T), avoiding the previous IndexError risk on ambiguous splits. - Add regression tests for both cases. Co-Authored-By: Claude Code --- .../datasources_and_workbooks_command.py | 7 ++++++- .../datasources_and_workbooks/export_command.py | 9 ++++++++- .../test_datasources_and_workbooks_command.py | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py index ae577bfa..ed89ac0e 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -103,7 +103,12 @@ def apply_encoded_filter_value(logger, request_options: RequestOptionsType, valu @staticmethod def apply_filter_value(logger, request_options: RequestOptionsType, value: str) -> None: logger.debug("handling filter param {}".format(value)) - data_filter = value.split("=") + # split on the first '=' only, so values that themselves contain '=' (e.g. "Notes=x=y") + # are preserved instead of being truncated. + data_filter = value.split("=", 1) + if len(data_filter) != 2: + logger.warning("Unable to read filter param '{}', expected COLUMN=VALUE. Skipping".format(value)) + return # we should export the _DataExportOptions class from tsc request_options.vf(data_filter[0], data_filter[1]) # type: ignore diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 78e32a61..23d32947 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -151,8 +151,15 @@ def apply_filters_from_args(request_options: RequestOptionsType, args, logger) - # with '&' (e.g. `--filter "a=1&b=2"`), so that one case still splits on '&'. # Repeated --filter flags each carry exactly one pair — no split — so a # literal '&' or '=' in a value is passed through untouched. + # Only treat '&' as a pair delimiter when every resulting segment looks like + # COLUMN=VALUE; otherwise the '&' is probably literal (e.g. inside a value like + # "Company=AT&T"), so leave it alone and pass the single filter through as-is. if len(args.filter) == 1: - values = args.filter[0].split("&") + candidates = args.filter[0].split("&") + if len(candidates) > 1 and all("=" in c for c in candidates): + values = candidates + else: + values = args.filter else: values = args.filter for value in values: diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 4afe2497..61926033 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -216,6 +216,22 @@ def test_apply_filters_from_args_backcompat_ampersand_join(self): ExportCommand.apply_filters_from_args(request_options, args, mock_logger) assert request_options.view_filters == [("Region", "West"), ("Product", "Widget")] + def test_apply_filters_from_args_single_with_literal_ampersand(self): + # Regression: a single --filter whose value contains a literal '&' but is not + # a '&'-joined COLUMN=VALUE pair list must not be split into bogus segments + # (which previously could raise IndexError downstream in apply_filter_value). + args = argparse.Namespace(filter=["Company=AT&T"]) + request_options = tsc.PDFRequestOptions() + ExportCommand.apply_filters_from_args(request_options, args, mock_logger) + assert request_options.view_filters == [("Company", "AT&T")] + + def test_apply_filters_from_args_preserves_equals_in_value(self): + # Regression: values containing '=' (e.g. "Notes=x=y") must not be truncated. + args = argparse.Namespace(filter=["Notes=x=y"]) + request_options = tsc.PDFRequestOptions() + ExportCommand.apply_filters_from_args(request_options, args, mock_logger) + assert request_options.view_filters == [("Notes", "x=y")] + @mock.patch("tableauserverclient.Server") class MockedServerTests(unittest.TestCase):