feat: support [default] section in config.toml for global flag defaults#7
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for a [default] section in config.toml to let users set persistent defaults for some CLI flags, and changes CLI parsing to apply config-derived values via argparse defaults.
Changes:
- Introduces a new
DefaultSectionin the config model and merges it into the argparse namespace. - Switches CLI parsing to use
parser.set_defaults()with config-derived values (instead of passing a pre-populated namespace). - Adds documentation describing the new config structure and example TOML.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| gcalcli/config.py | Adds [default] section to the Pydantic config model and includes it in to_argparse_namespace(). |
| gcalcli/cli.py | Applies config values as argparse defaults via parser.set_defaults() before the final parse. |
| gcalcli/argparsers.py | Updates help text for --iamaexpert. |
| docs/configuration.md | New documentation for config.toml, including [default] section examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| interactive: bool = Field( | ||
| alias='interactive', | ||
| title='Enable interactive mode (default: true)', | ||
| default=True | ||
| ) |
There was a problem hiding this comment.
interactive is introduced as a configurable value, but it doesn't correspond to any argparse option (parsed_args.interactive is never referenced) and therefore has no effect. Either wire it to an existing flag (e.g. delete --iamaexpert / add a real --yes flag, or map it to a prompt/noprompt behavior), or remove/rename this field to match an actually-consumed option name.
| interactive: bool = Field( | |
| alias='interactive', | |
| title='Enable interactive mode (default: true)', | |
| default=True | |
| ) |
| [default] | ||
| interactive = false # Disable interactive prompts (simulates --yes) | ||
| color = true # Enable/Disable color | ||
| conky = false # Conky compatible output |
There was a problem hiding this comment.
This section documents interactive = false as “simulates --yes”, but there is no --yes flag in the current CLI (only delete --iamaexpert exists) and the newly-added interactive config key is not used by the application. Update the documentation to describe the actual supported flags/behavior, or implement the promised --yes/non-interactive behavior.
| ) | ||
| delete.add_argument( | ||
| '--iamaexpert', action='store_true', help='Probably not' | ||
| '--iamaexpert', action='store_true', help='Legacy alias for --yes' |
There was a problem hiding this comment.
The help text says this is a legacy alias for --yes, but --yes is not defined anywhere in the argument parser. Either add a real --yes option (and make --iamaexpert an alias), or adjust the help text to describe the actual behavior (skips the delete confirmation prompt).
| '--iamaexpert', action='store_true', help='Legacy alias for --yes' | |
| '--iamaexpert', | |
| action='store_true', | |
| help='Skip delete confirmation prompt (legacy expert flag)', |
| # Apply config values as defaults for the parser. | ||
| # This allows config to override program defaults, while still letting | ||
| # explicit CLI arguments override config. | ||
| parser.set_defaults(**vars(namespace_from_config)) | ||
|
|
||
| if parsed_args.includeRc: | ||
| argv = fromfile_args + argv | ||
| (parsed_args, unparsed) = parser.parse_known_args( | ||
| argv, namespace=namespace_from_config | ||
| ) | ||
| if parsed_args.week_start is None: | ||
| parsed_args.week_start = week_start | ||
| (parsed_args, unparsed) = parser.parse_known_args(argv) | ||
|
|
There was a problem hiding this comment.
This changes how config values are applied to argparse (switching to set_defaults). There are existing parser unit tests in tests/test_argparsers.py, but no tests asserting that values from config.toml become parser defaults and that explicit CLI args still win (e.g., week_start and boolean flags like --nocolor). Adding a targeted test would help prevent regressions in this precedence logic.
| @@ -98,6 +123,8 @@ def to_argparse_namespace(self) -> argparse.Namespace: | |||
| kwargs.update(vars(self.calendars)) | |||
| if self.output: | |||
| kwargs.update(vars(self.output)) | |||
| if self.default: | |||
| kwargs.update(vars(self.default)) | |||
| return argparse.Namespace(**kwargs) | |||
There was a problem hiding this comment.
Config now supports a [default] section and merges it into the argparse namespace. There are currently no unit tests covering TOML parsing for this new section (or verifying that its values appear in to_argparse_namespace()), despite the repository having a tests/ suite. Please add a test that loads a minimal TOML with [default] and asserts the resulting namespace contains the expected values.
Ported from insanum/gcalcli#863 by @Aurelian-Shuttleworth.
Summary
[default]section to config.toml model withinteractive,color,conkyfieldsparser.set_defaults()to apply config values, letting explicit CLI args still overridedocs/configuration.mdwith example configWhy
Reduces need for shell aliases. Users can set permanent defaults (e.g.
color = false) without modifying shell config.🤖 Generated with Claude Code