Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double negatives on bool opts with underscores #551

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tommarshall
Copy link

Because:

  • Negative options #460 (released in 0.19.2) introduced a change that prevents double negative options (--no-no-foo) for already negative boolean options (--no-foo).
  • That change checks if the option name string starts with 'no-', before it's dasherized, therefore negative options defined with underscores (:no_foo) still generate a double negative --no-no-foo option.

This change:

  • Adds a test for, and fixes the double negative boolean options
    defined with underscores.

Because:

* rails#460 (released in `0.19.2`)
  introduced a change that prevents double negative options
  (`--no-no-foo`) for already negative  boolean options (`--no-foo`).
* That change checks if the option name string starts with 'no-', before
  it's `dasherized`, therefore negative options defined with underscores
  (`:no_foo`) still generate a double negative `--no-no-foo` option.

This change:

* Adds a test for, and fixes the double negative boolean options
  defined with underscores.
@piotrmurach
Copy link

This is related to #553 in as far as negation of boolean flags is concerned. I feel as though this automatic negation provides more obscure behaviour than real benefits. Your fix kind of demonstrates the possible corner cases etc.... I'm not sure if this should be merged or the behaviour of actual negation changed. Teetering towards the later.

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