thor: Returns exit 0 when a required argument is not provided

Ok, so we bump the major version of Thor, and put it in the changelog. That’s what semantic versioning is for, right?

I raised a similar issue directly with Bundler, who’ve referred me here. It’s as simple as if you can’t do what I ask, don’t exit 0 – anything else is a bastardisation of 30 years of software sane practice.

https://github.com/carlhuda/bundler/pull/1892#issuecomment-15928663

Ok, so we bump the major version of Thor, and put it in the changelog. That’s what semantic versioning is for, right?

It is indeed. The problem with Thor is that it’s never reached the 1.0 version, which is what SemVer defines as:

1. Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.
2. Version 1.0.0 defines the public API. The way in which the version number is incremented after this release is dependent on this public API and how it changes.

So, we have a version which is really defined for “initial development” even though it has been in widespread production usage for years. This is of course not optimal, but we must also remember and accept that when Thor was initially created, the SemVer specification as we know it today didn’t even exist. Things are a lot different now than they used to be back then.

Anyhow, I do agree with your point: we should work towards a 1.0 release of Thor, so we can start applying reasonable SemVer semantics to the releases etc.

My suggestion (this is mostly directed to @wycats or someone else working with the maintenance of this gem):

• Bump the version of the current master from 0.19.4 to 1.0.0 straight away, and publish it as a Rubygem. Make it a public announcement that the public API is now to be trusted.
• Start working on the next major version, i.e. 2.0.0. In that version, small annoyances like this (which I definitely agree is a bad design decision, probably with historical reasons behind it) can be smoothed over.

Hand raised ✋: I am willing to volunteer in this as needed, but is not currently a contributor or maintainer of this gem. If help/PRs are requested to get this moving, please let me and others know and we’ll do our best to contribute back to a very valuable Rubygem that means a lot to many of us.

I guess the current, correct approach is similar to:

class MyStuff < Thor
  def self.exit_on_failure?
    true
  end

  desc "epic_failure", "failure is epic"
  def epic_failure
    raise Thor::Error, "E-E-E-EPIC-C-C-c-c-c-c-.-.-."
  end
end

$ thor my_stuff:epic_failure
E-E-E-EPIC-C-C-c-c-c-c-.-.-.
$ echo $?
1

I think it just needs to be more clearly documented somewhere…

Two thumbs up reacts and I’ll make a PR to stick this in the repo’s README.

@mohkale Currently if you run the program in the initial post of this ticket, you get a useful deprecation warning:

./test.rb foo -output=blah
No value provided for required options '--output'
Deprecation warning: Thor exit with status 0 on errors. To keep this behavior, you must define `exit_on_failure?` in `Foo`
You can silence deprecations warning by setting the environment variable THOR_SILENCE_DEPRECATION.

So this should get fully fixed in the next major release (I guess), and you can already opt-in to the right behaviour very easily.

i guess this will never get implemented, eventhough that is pretty critical?

The case https://github.com/erikhuda/thor/issues/244#issue-6116190 is still not working, even with adding

def self.exit_on_failure?
    true
  end

I guess the current, correct approach is similar to:

class MyStuff < Thor
  def self.exit_on_failure?
    true
  end

  desc "epic_failure", "failure is epic"
  def epic_failure
    raise Thor::Error, "E-E-E-EPIC-C-C-c-c-c-c-.-.-."
  end
end

$ thor my_stuff:epic_failure
E-E-E-EPIC-C-C-c-c-c-c-.-.-.
$ echo $?
1

I think it just needs to be more clearly documented somewhere…