flag_or_value_option type: [TrueClass, String] is too narrow for action-option-with-optional-value commands #1200
Description
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)—Symbolis not in[TrueClass, String], even though:diff.to_sproduces 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
-
Add a
reject:modifier — e.g.reject: [FalseClass]— that blacklists specific types instead of whitelisting. Thetype:constraint would no longer be needed for this pattern. -
Widen the type list — e.g.
type: [TrueClass, String, Symbol]— but this is fragile and doesn't cover all#to_s-capable types. -
Introduce a default false-rejection for
flag_or_value_option— iffalseis never a valid input forflag_or_value_option, the DSL method could reject it automatically without requiringtype:.
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.rb—type: [TrueClass, String]lib/git/commands/push.rb—type: [String, FalseClass]onrecurse_submoduleslib/git/commands/commit.rb—type: Stringondate(less impactful but also too narrow)
Activity