Bug fix for issue 128, plus useful extensions #147

Open
wants to merge 7 commits into
from

Conversation

Projects
None yet
4 participants
@rleber

rleber commented Jul 4, 2011

This replaces pull request at issue 143. It includes a bug fix for issue 128 and some changes to add a few capabilities I've found helpful:

  • Adds a banner method for a Thor class (not just a banner for an individual task)
  • Allows default tasks to take arguments (so, if the class foo has default task bar and the user specifies thor foo plugh where plugh is not a recognized task, it is interpreted as thor foo bar plugh
  • Improve error-handling if the default task is not found (e.g. if default_task :chewbacca is specified, but there is no "chewbacca" task)
  • Fixes the bug in Issue 128 related to listing subcommands (and replaces the patch I submitted as Issue 137). The fix is in commit 0726f56
  • Allows for repeating options, e.g. thor class task -r file1 -r file2
  • Fixes a small bug in print_wrapped (occasionally did not handle \005 and \177 correctly)

This fixes the bugs in the original submission of this patch It passes all tests ONCE YOU FIX THE BUG identified in issue 144. See my patch submitted separately as issue 146.

Thanks again to everyone else whose work made Thor so terrific.

rleber added some commits Jun 18, 2011

Fix problem with help not including parent command names for subcommands
See problem identified in github Issue 128. This patch has been submitted
as Issue 137
Rename banner as task_banner (to distinguish it from overall banners for
Thor classes). Add Thor.banner method to set overall banner for a Thor
class. Add tests for Thor.banner
Allow Thor to specify using a default task with arguments, if argumen…
…ts match no method, i.e.

  thor klass blah # where blah is not a task, defaults to: thor klass default_task blah

Also add tests for default tasks with arguments, and fix typo in documentation comments for default_task
@sferik

This comment has been minimized.

Show comment Hide comment
@sferik

sferik Jul 4, 2011

Member

This looks good to me. Any other comments before we merge?

Member

sferik commented Jul 4, 2011

This looks good to me. Any other comments before we merge?

@eventualbuddha

This comment has been minimized.

Show comment Hide comment
@eventualbuddha

eventualbuddha Jul 7, 2011

Contributor

Has this been tested with any of the important uses of thor, like rails or bundler?

Contributor

eventualbuddha commented Jul 7, 2011

Has this been tested with any of the important uses of thor, like rails or bundler?

@wycats

This comment has been minimized.

Show comment Hide comment
@wycats

wycats Nov 21, 2011

Member

Additionally, can you rebase. I pulled a different fix for 128 before I saw this.

Member

wycats commented Nov 21, 2011

Additionally, can you rebase. I pulled a different fix for 128 before I saw this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment