Allow to suppress automatic creation of --no-boolean flags

[xjunior]

Adds inverse: (false|SymbolString) option to suppress automatic creation of --no-[option] flags or customize the flag name.

Closes #417

[xjunior]

@rafaelfranca hello, can we get this in?

[dorner]

@xjunior there was some interest in being able to customize the inverse behavior. So rather than just "on/off" I'd like to see something like "inverse: 'inverse-name'" which would default to "no-{name}" but could be set to false to disable the inverse behavior entirely. Does that make sense?

[xjunior]

@dorner how does this look now?

[dorner]

Looking good! Couple of suggestions.

[xjunior]

@dorner done and done

[dorner]

Much nicer! @rafaelfranca you good?

[rafaelfranca]

Don't this patch only change the usage output? I don't see the @inverse value being used to calculate the value.

[xjunior]

@rafaelfranca the inverse flags are really aesthetic apparently, as the actual option (true value) is what is really checked.

i.e.:

option :color, inverse: :no_color
...
use_color! if options[:color]

[rafaelfranca]

They do have behavior. Here for example. https://github.com/erikhuda/thor/blob/011dc48b5ea92767445b062f971664235973c8b4/lib/thor/parser/options.rb#L179. Your patch don't disable that behavior, only the help message.

[G-Rath]

Bit of a drive-by comment: once this is landed, would folks be open to supporting using :inverse on non-boolean types as well? A common use-case I have for this is with flags like --config, where I'd like to have --no-config to allow users to explicitly signify that the flag (including its default) should not be used (i.e "do not read any config file, even if the default .my-tools-config.json file is present")

Happy to make a new issue for this and even have a go at the PR, but figured it'd be better to wait until this is merged :)

[G-Rath]

@xjunior I'd also be open to helping with this PR, if you'd like a hand.

[dorner]

I'd be amenable to non-boolean inverse, but I'd like to see the PR to understand your approach.

[xjunior]

Quite honestly, my interest in this was sorely to disable the feature when necessary. Customizing the inverse name, and adding inverse for non-boolean flags touches the code a bit more as shown by @rafaelfranca. Unfortunately I haven't been having the capacity to work more on this, so if @G-Rath wants to pick this up, you're more than welcome :D Otherwise, I think I'm rolling back the changes to customize the name and keep the original intent of the PR. Thoughts?

[dorner]

I think the two pieces of functionality are separate enough that they should be two different PRs.

[G-Rath]

Customizing the inverse name, and adding inverse for non-boolean flags touches the code a bit more as shown
I think the two pieces of functionality are separate enough that they should be two different PRs.

Completely agree - I think it would be best to land things in three sequential PRs:

1. add inverse: true|false on boolean type flags to allow disabling the current --no-<flag> behaviour
2. support inverse: true|false|<string> on boolean type flags to allow customising the inverse flag
3. support inverse: true|false|<string> on non-boolean type flags

@dorner would you be open to doing it this way?

[xjunior]

@rafaelfranca

They do have behavior. Here for example. https://github.com/erikhuda/thor/blob/011dc48b5ea92767445b062f971664235973c8b4/lib/thor/parser/options.rb#L179. Your patch don't disable that behavior, only the help message.

I tried to follow the code, and I really couldn't see how it would influence the outcome. Can you provide an example?

[monfresh]

@rafaelfranca Following up on this. Could you please explain what you mean by the need to disable the behavior? Do you mean that if this option exists:

option :run, type: :boolean, default: false, inverse: false

then if someone tries to pass in the --no-run option, it should fail with an error like this one?

ERROR: "foo command" was called with arguments ["--no-run"]
Usage: "foo command"

[rafaelfranca]

then if someone tries to pass in the --no-run option, it should fail with an error like this one?

Yes, but only if no invalidation options are accepted. But mainly the value of run should not be false.

[xjunior]

Hello, @rafaelfranca. Would you give me some hints on how to implement this behavior?