Skip to content

flag_or_value_option type: [TrueClass, String] is too narrow for action-option-with-optional-value commands #1200

@jcouball

Description

@jcouball

Problem

flag_or_value_option accepts any object with a meaningful #to_s implementation by default — String, Integer, Float, Pathname, Symbol, etc. — and stringifies it automatically during the build phase. However, when type: [TrueClass, String] is added (as required for action-option-with-optional-value commands to prevent false from silently emitting nothing), it over-constrains the input by rejecting valid #to_s-capable values like Symbol.

Example from Git::Commands::Am::ShowCurrentPatch:

flag_or_value_option :show_current_patch, inline: true, type: [TrueClass, String]

This correctly prevents false from passing through silently, but rejects:

  • cmd.call(:diff)Symbol is not in [TrueClass, String], even though :diff.to_s produces the valid CLI argument "diff"
  • cmd.call(Pathname.new('diff')) — similarly rejected despite producing correct output

The same pattern exists in Git::Commands::Push (type: [String, FalseClass] on recurse_submodules).

Root cause

The type: modifier validates the Ruby class of the input before #to_s stringification. For action-option-with-optional-value commands, the real need is narrower: reject false (which would silently emit nothing) while allowing anything that stringifies correctly.

Possible solutions

  1. Add a reject: modifier — e.g. reject: [FalseClass] — that blacklists specific types instead of whitelisting. The type: constraint would no longer be needed for this pattern.

  2. Widen the type list — e.g. type: [TrueClass, String, Symbol] — but this is fragile and doesn't cover all #to_s-capable types.

  3. Introduce a default false-rejection for flag_or_value_option — if false is never a valid input for flag_or_value_option, the DSL method could reject it automatically without requiring type:.

Option 3 is probably the cleanest: flag_or_value_option semantically means "flag or value" — false (meaning "don't emit the flag") is arguably not a valid state for this method. If callers don't want the flag, they should omit the keyword entirely (or pass nil).

Affected commands

  • lib/git/commands/am/show_current_patch.rbtype: [TrueClass, String]
  • lib/git/commands/push.rbtype: [String, FalseClass] on recurse_submodules
  • lib/git/commands/commit.rbtype: String on date (less impactful but also too narrow)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions