Skip to content

Conversation

@reggi
Copy link
Contributor

@reggi reggi commented Nov 24, 2025

Command-Specific Flags Implementation Checklist

This PR enables commands to define their own distinct flags separate from global configuration, eliminating the need for verbose prefixed flags like --init-name and allowing cleaner command-specific options.

Key Features

  • New flags configuration source - Adds a dedicated layer in the config hierarchy for command-specific flags without modifying existing sources
  • Command definition merging - Implements loadCommand() method to merge command-specific definitions with global definitions
  • Warning deduplication - Captures and deduplicates warnings during initial config load to prevent duplicate messages across sources
  • Synchronous design - No new async calls or config instances, integrates seamlessly with existing load flow
  • Clean flag naming - Enables commands to use simple flags (e.g., --name) instead of verbose prefixed versions (e.g., --init-name)

Implementation Tasks

  • Add flags to config source hierarchy (between env and cli)
  • Implement loadCommand(definitions) method for dynamic definition merging
  • Add warning capture system (#warnings array) to delay logging until after command load
  • Implement removeWarnings(types) to clear superseded warnings
  • Implement #deduplicateWarnings() to prevent duplicate warnings
  • Add logWarnings() method to flush captured warnings
  • Update type/default merging to support command-specific definitions
  • Handle exclusive flag validation across merged definitions
  • Update checkUnknown() to recognize both cli and flags sources
  • Test command-specific flag parsing and precedence

@reggi reggi requested a review from a team as a code owner November 24, 2025 14:21
@reggi reggi marked this pull request as draft November 24, 2025 14:31
@reggi reggi changed the title Command Configs chore: Command Configs Nov 27, 2025
@reggi reggi marked this pull request as ready for review December 1, 2025 17:33
@reggi reggi requested a review from wraithgar December 2, 2025 22:26
Comment on lines +24 to +31
if (this.definitions) {
this.definitions = this.definitions
} else {
this.definitions = definitions
}

const definitionsPool = { ...definitions, ...this.definitions }

Copy link
Member

Choose a reason for hiding this comment

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

This part seems weird to me, we're merging potentially both into definitionsPool, with this.definitions taking predecent. But if there are no this.definitions then they are both the same value so why do we care about both. This probably needs a quick boolean tree to see what to do in all 4 cases (has/doesn't have definitions and has/doesn't have this.definitions)

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