Skip to content

fix: keep global options given before the subcommand - #493

Open
jacalata wants to merge 3 commits into
developmentfrom
jac/global-options-before-subcommand
Open

jacalata wants to merge 3 commits into
developmentfrom
jac/global-options-before-subcommand

Conversation

@jacalata

@jacalata jacalata commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Global options placed before the subcommand were silently dropped, so tabcmd -s <url> --token-name n --token-value v login ran with no server or credentials, and tabcmd --language de login ignored the language. Only options placed after the subcommand worked.

The root parser and every subcommand parser shared one set of global options with defaults. When a subcommand parser ran, its defaults overwrote the values already parsed before the subcommand. The subcommand parsers now use a copy of the global options with defaults suppressed; the root parser keeps the real defaults, so every option still has a default value. Options given after the subcommand still win over ones given before it.

Tests build the real parser and check every global option gives the same result before and after the subcommand, across several commands and help. Help output is unchanged.

🤖 Generated with Claude Code

jacalata and others added 2 commits November 3, 2026 14:59
The root parser and every subparser share the same global options, so
the subparser's defaults overwrote values given before the subcommand:
`tabcmd --server X --language de login` lost both. Build the
subcommand copy of the global options with defaults suppressed so a
value from either position is kept, and a value after the subcommand
still wins.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Covers every global option before and after the subcommand, boolean flag
defaults, converters, the full default set, and the help command.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 4, 2026 00:54
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
tabcmd
   __main__.py121212 0%
   _version.py111111 0%
   tabcmd.py151515 0%
   version.py1066 40%
tabcmd/commands
   commands.py101010 0%
   constants.py771515 81%
   server.py1351818 87%
tabcmd/commands/auth
   session.py4184646 89%
tabcmd/commands/datasources_and_workbooks
   datasources_and_workbooks_command.py1701818 89%
   datasources_workbooks_views_url_parser.py14255 96%
   delete_command.py702323 67%
   export_command.py1262121 83%
   get_url_command.py1274747 63%
   publish_command.py1341616 88%
   runschedule_command.py2177 67%
tabcmd/commands/extracts
   create_extracts_command.py4288 81%
   decrypt_extracts_command.py2722 93%
   delete_extracts_command.py3766 84%
   encrypt_extracts_command.py2722 93%
   extracts.py2022 90%
   reencrypt_extracts_command.py2722 93%
   refresh_extracts_command.py481010 79%
tabcmd/commands/group
   create_group_command.py2955 83%
   delete_group_command.py2722 93%
tabcmd/commands/project
   create_project_command.py4688 83%
   delete_project_command.py3544 89%
   publish_samples_command.py3044 87%
tabcmd/commands/site
   create_site_command.py3455 85%
   delete_site_command.py2722 93%
   edit_site_command.py661818 73%
   list_command.py771212 84%
   list_sites_command.py2922 93%
tabcmd/commands/user
   add_users_command.py2955 83%
   create_site_users.py1151414 88%
   create_users_command.py5999 85%
   delete_site_users_command.py4355 88%
   user_data.py2223131 86%
tabcmd/execution
   _version.py222 0%
   global_options.py15544 97%
   localize.py671111 84%
   logger_config.py7266 92%
   tabcmd_controller.py4077 82%
TOTAL306845885% 

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Mutually exclusive global options bypass conflict validation when split across the subcommand, allowing an explicitly supplied proxy to be silently overridden and similar credential and certificate conflicts to go undetected.

1 open issue
What changed in this PR

This PR fixes global options being lost when supplied before a subcommand, while preserving root-level defaults and allowing later values to take precedence.

Changes:

  • Gives subcommands and help separate global options with defaults suppressed.
  • Adds parser regression tests for option placement, value conversion, defaults, and precedence.
File Description
tests/​parsers/​test_global_options_position.py Tests global option handling across multiple commands and help.
tabcmd/​execution/​parent_parser.py Prevents subcommand defaults from overwriting previously parsed global options.

Review effort: Balanced

Comment thread tabcmd/execution/parent_parser.py Resolved
Global options can be given before and after the subcommand, and argparse
only enforces mutually exclusive groups within one parser, so
'tabcmd --no-proxy login --proxy host:1' kept both and --no-proxy silently
won. Check the pair in Session, where the connection is configured.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 7, 2026 06:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the CLI option-precedence and proxy-validation changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants