From d1024e752e35d8940c69e77af5af927942b6b9e0 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 3 Aug 2026 18:10:36 -0700 Subject: [PATCH 1/3] Fix #109: connect --language and --country to export request options --language was already threading through to request_options.language on export, but --country never reached the REST API. tabcmd Classic combined the two into a BCP 47 locale like en-GB or de-DE so exports formatted numbers, dates, and currency for that region. - Add _resolve_locale() that joins language + country when both are set, falls back to language alone, and drops country-only (matching Classic which required --language with --country). - Route apply_png_options / apply_pdf_options / apply_csv_options through the helper. - Relax --country choices from language codes to any 2-letter code (upper-cased), so users can pass real country codes like GB, MX, BR. Fixes #109. --- .../datasources_and_workbooks_command.py | 28 +++++++++++---- tabcmd/execution/parent_parser.py | 6 ++-- .../test_datasources_and_workbooks_command.py | 35 +++++++++++++++++++ 3 files changed, 61 insertions(+), 8 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..3629f64c 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -146,6 +146,19 @@ def apply_options_in_url(logger, request_options: RequestOptionsType, value: str def is_truthy(value: str): return value.lower() in ["yes", "y", "1", "true"] + @staticmethod + def _resolve_locale(args): + # tabcmd Classic accepts --language and --country to control + # export locale. tabcmd 2 exposes both as global flags but only --language + # was reaching the REST API. Combine them into a BCP 47 locale when both + # are given; --country alone is treated as an incomplete locale and warned + # about, matching Classic which required --language with --country. + language = getattr(args, "language", None) + country = getattr(args, "country", None) + if language and country: + return "{}-{}".format(language, country) + return language + @staticmethod def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): # these are only used in export, not get @@ -157,8 +170,9 @@ def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): request_options.image_resolution = None else: request_options.image_resolution = TSC.ImageRequestOptions.Resolution.High.lower() - if args.language: - request_options.language = args.language + locale = DatasourcesAndWorkbooks._resolve_locale(args) + if locale: + request_options.language = locale @staticmethod def apply_pdf_options(logger, request_options: TSC.PDFRequestOptions, args): @@ -170,13 +184,15 @@ def apply_pdf_options(logger, request_options: TSC.PDFRequestOptions, args): request_options.viz_height = int(args.height) if args.width: request_options.viz_width = int(args.width) - if args.language: - request_options.language = args.language + locale = DatasourcesAndWorkbooks._resolve_locale(args) + if locale: + request_options.language = locale @staticmethod def apply_csv_options(logger, request_options: TSC.CSVRequestOptions, args): - if args.language: - request_options.language = args.language + locale = DatasourcesAndWorkbooks._resolve_locale(args) + if locale: + request_options.language = locale @staticmethod def save_to_data_file(logger, output, filename): diff --git a/tabcmd/execution/parent_parser.py b/tabcmd/execution/parent_parser.py index 2a4183e7..c2e10d30 100644 --- a/tabcmd/execution/parent_parser.py +++ b/tabcmd/execution/parent_parser.py @@ -103,8 +103,10 @@ def parent_parser_with_global_options(): parser.add_argument( "--country", - choices=["de", "en", "es", "fr", "it", "ja", "ko", "pt", "sv", "zh"], - type=str.lower, # coerce input to lowercase to act case insensitive + # ISO 3166-1 alpha-2 country code (case-insensitive). Combined with --language + # to form a locale (e.g. --language en --country GB -> "en-GB"). Left + # unconstrained on choices since Classic accepts any 2-letter country code. + type=str.upper, help=_("export.options.country"), ) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 8fa60dde..b02bf7f7 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -185,10 +185,45 @@ def test_apply_csv_options(self): def test_apply_csv_options_with_language(self): mock_args.language = "de" + mock_args.country = None request_options = tsc.CSVRequestOptions() DatasourcesAndWorkbooks.apply_csv_options(mock_logger, request_options, mock_args) assert request_options.language == "de" + def test_apply_csv_options_with_language_and_country(self): + # tabcmd Classic combined --language + --country into a locale (issue #109). + local_args = argparse.Namespace(language="en", country="GB") + request_options = tsc.CSVRequestOptions() + DatasourcesAndWorkbooks.apply_csv_options(mock_logger, request_options, local_args) + assert request_options.language == "en-GB" + + def test_apply_png_options_with_language_and_country(self): + local_args = argparse.Namespace( + language="fr", country="CA", height="600", width="800", resolution=None + ) + request_options = tsc.ImageRequestOptions() + DatasourcesAndWorkbooks.apply_png_options(mock_logger, request_options, local_args) + assert request_options.language == "fr-CA" + + def test_apply_pdf_options_with_language_and_country(self): + local_args = argparse.Namespace( + language="pt", country="BR", height=800, width=600, + pagelayout=tsc.PDFRequestOptions.Orientation.Portrait.__str__(), + pagesize=tsc.PDFRequestOptions.PageType.Folio.__str__(), + ) + request_options = tsc.PDFRequestOptions() + DatasourcesAndWorkbooks.apply_pdf_options(mock_logger, request_options, local_args) + assert request_options.language == "pt-BR" + + def test_resolve_locale_country_without_language_is_dropped(self): + # --country alone is not a valid locale; drop it and don't send to server. + local_args = argparse.Namespace(language=None, country="US") + assert DatasourcesAndWorkbooks._resolve_locale(local_args) is None + + def test_resolve_locale_language_only(self): + local_args = argparse.Namespace(language="ja", country=None) + assert DatasourcesAndWorkbooks._resolve_locale(local_args) == "ja" + @mock.patch("tableauserverclient.Server") class MockedServerTests(unittest.TestCase): From f3f9af4daefb15f72e70506c19e5f3362f405087 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Fri, 7 Aug 2026 00:00:30 -0700 Subject: [PATCH 2/3] style: apply black to satisfy CI --- tests/commands/test_datasources_and_workbooks_command.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index b02bf7f7..28654041 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -198,16 +198,17 @@ def test_apply_csv_options_with_language_and_country(self): assert request_options.language == "en-GB" def test_apply_png_options_with_language_and_country(self): - local_args = argparse.Namespace( - language="fr", country="CA", height="600", width="800", resolution=None - ) + local_args = argparse.Namespace(language="fr", country="CA", height="600", width="800", resolution=None) request_options = tsc.ImageRequestOptions() DatasourcesAndWorkbooks.apply_png_options(mock_logger, request_options, local_args) assert request_options.language == "fr-CA" def test_apply_pdf_options_with_language_and_country(self): local_args = argparse.Namespace( - language="pt", country="BR", height=800, width=600, + language="pt", + country="BR", + height=800, + width=600, pagelayout=tsc.PDFRequestOptions.Orientation.Portrait.__str__(), pagesize=tsc.PDFRequestOptions.PageType.Folio.__str__(), ) From 089eb0b8f23935715d0c6d6f96c0f2e4b809a10f Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Sat, 8 Aug 2026 01:15:52 -0700 Subject: [PATCH 3/3] Actually warn on --country without --language The docstring/PR body claimed --country alone was warned about, but the code silently dropped it. Thread the logger through _resolve_locale and emit a warning that names the ignored country code. Adds tests for both the warn-and-drop and don't-warn-when-language-is-set paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../datasources_and_workbooks_command.py | 10 ++++++---- tabcmd/locales/en/tabcmd_messages_en.properties | 1 + .../test_datasources_and_workbooks_command.py | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 4 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 3629f64c..1780b245 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -147,7 +147,7 @@ def is_truthy(value: str): return value.lower() in ["yes", "y", "1", "true"] @staticmethod - def _resolve_locale(args): + def _resolve_locale(args, logger=None): # tabcmd Classic accepts --language and --country to control # export locale. tabcmd 2 exposes both as global flags but only --language # was reaching the REST API. Combine them into a BCP 47 locale when both @@ -157,6 +157,8 @@ def _resolve_locale(args): country = getattr(args, "country", None) if language and country: return "{}-{}".format(language, country) + if country and not language and logger is not None: + logger.warning(_("export.locale.country_without_language").format(country)) return language @staticmethod @@ -170,7 +172,7 @@ def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): request_options.image_resolution = None else: request_options.image_resolution = TSC.ImageRequestOptions.Resolution.High.lower() - locale = DatasourcesAndWorkbooks._resolve_locale(args) + locale = DatasourcesAndWorkbooks._resolve_locale(args, logger) if locale: request_options.language = locale @@ -184,13 +186,13 @@ def apply_pdf_options(logger, request_options: TSC.PDFRequestOptions, args): request_options.viz_height = int(args.height) if args.width: request_options.viz_width = int(args.width) - locale = DatasourcesAndWorkbooks._resolve_locale(args) + locale = DatasourcesAndWorkbooks._resolve_locale(args, logger) if locale: request_options.language = locale @staticmethod def apply_csv_options(logger, request_options: TSC.CSVRequestOptions, args): - locale = DatasourcesAndWorkbooks._resolve_locale(args) + locale = DatasourcesAndWorkbooks._resolve_locale(args, logger) if locale: request_options.language = locale diff --git a/tabcmd/locales/en/tabcmd_messages_en.properties b/tabcmd/locales/en/tabcmd_messages_en.properties index 7eda4bff..ad1dfefd 100644 --- a/tabcmd/locales/en/tabcmd_messages_en.properties +++ b/tabcmd/locales/en/tabcmd_messages_en.properties @@ -51,6 +51,7 @@ encryptextracts.status=Scheduling extracts on site {0} to be encrypted... export.errors.white_space_workbook_view=The name of the workbook or view to export cannot include spaces. Use the normalized name of the workbook or view as it appears in the URL. export.errors.requires_workbook_view_param=The ''{0}'' command requires a / parameter, and there must be at least one slash (/) in this parameter export.errors.requires_valid_custom_view_uuid=The URL for custom views must contain a valid custom view uuid +export.locale.country_without_language=--country {0} was ignored: --country requires --language (e.g. --language en --country GB). Using the site''s default locale. export.options.country=If not using user''s default locale, the country abbreviation for locale (find in IANA Language Subtag Registry). Must use with --language export.options.csv=Export data in CSV format (default) export.options.fullpdf=Export visual views in PDF format (if workbook was published with tabs) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 28654041..f93e2b16 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -221,10 +221,26 @@ def test_resolve_locale_country_without_language_is_dropped(self): local_args = argparse.Namespace(language=None, country="US") assert DatasourcesAndWorkbooks._resolve_locale(local_args) is None + def test_resolve_locale_country_without_language_warns(self): + # The user's --country was silently dropped in prior revisions; make sure + # they get a warning. In tests the gettext catalog isn't loaded so `_()` + # returns the raw key -- assert on the key rather than the English text. + local_args = argparse.Namespace(language=None, country="US") + warn_logger = mock.MagicMock() + assert DatasourcesAndWorkbooks._resolve_locale(local_args, warn_logger) is None + warn_logger.warning.assert_called_once() + assert "country_without_language" in warn_logger.warning.call_args[0][0] + def test_resolve_locale_language_only(self): local_args = argparse.Namespace(language="ja", country=None) assert DatasourcesAndWorkbooks._resolve_locale(local_args) == "ja" + def test_resolve_locale_language_only_does_not_warn(self): + local_args = argparse.Namespace(language="ja", country=None) + warn_logger = mock.MagicMock() + DatasourcesAndWorkbooks._resolve_locale(local_args, warn_logger) + warn_logger.warning.assert_not_called() + @mock.patch("tableauserverclient.Server") class MockedServerTests(unittest.TestCase):