jwaldrip / admiral.cr

A robust DSL for writing command line interfaces written in Crystal.
https://jwaldrip.github.com/admiral.cr
MIT License
135 stars 14 forks source link

Required arguments not working #14

Closed ravern closed 4 years ago

ravern commented 5 years ago

Required arguments don't raise an error when no argument is passed.

Add the following line at the bottom of examples/complex.cr to reproduce this bug.

Complex.run ""

There is no error even though the argument required, which is required, is not passed. The argument is defined below.

  define_argument required : Int32, required: true

This is my shards.yaml.

name: kato
version: 0.1.0

authors:
  - Ravern Koh <ravernkoh@gmail.com>

description: |
  Toy virtual machine written in Crystal

targets:
  kato:
    main: src/kato/cli.cr

dependencies:
  admiral:
    github: jwaldrip/admiral.cr

development_dependencies:
  cake:
    git: https://github.com/ravernkoh/cake.git
    branch: master

license: GPL-3.0

Edit: Made a mistake in the example to reproduce, was originally Complex.run —required woah”.

s0kil commented 5 years ago

Try Complex.run "woah", when specifying -- it is a flag instead of an argument.

https://github.com/jwaldrip/admiral.cr#simple-flags https://github.com/jwaldrip/admiral.cr#arguments

ravern commented 5 years ago

Oh oops I made a mistake in the example 😅.

I actually meant Complex.run “”. I will edit the domain description.

s0kil commented 5 years ago
require "admiral"

class Hello < Admiral::Command
  define_help description: "A command that says hello"
  define_argument planet : String, required: true

  def run
    puts "Hello #{arguments.planet}"
  end
end

Hello.run

crystal hello.cr world Outputs: Hello world crystal hello.cr Throws an error

It seems to work as intended.

I think Complex.run “” still passes, as an empty string.

ravern commented 5 years ago

I ran your code.

The error raised is the following.

Unhandled exception: Nil assertion failed (Exception)
  from /usr/local/Cellar/crystal/0.25.1/src/nil.cr:106:5 in 'not_nil!'
  from src/hello.cr:0:3 in 'planet'
  from src/hello.cr:8:10 in 'run'
  from src/hello.cr:3:1 in 'parse_and_run'
  from src/hello.cr:3:1 in 'run'
  from src/hello.cr:12:1 in '__crystal_main'
  from /usr/local/Cellar/crystal/0.25.1/src/crystal/main.cr:104:5 in 'main_user_code'
  from /usr/local/Cellar/crystal/0.25.1/src/crystal/main.cr:93:7 in 'main'
  from /usr/local/Cellar/crystal/0.25.1/src/crystal/main.cr:133:3 in 'main'

However, if when I change it to a required flag, the following error is printed.

Flag required: --planet

Is this behaviour intended? I personally would expect the following to be printed if a required argument is not provided.

Argument required: planet

The edited code for the flag is here.

require "admiral"

class Hello < Admiral::Command
  define_help description: "A command that says hello"
  define_flag planet : String, required: true

  def run
    puts "Hello #{flags.planet}"
  end
end

Hello.run
s0kil commented 5 years ago

You are running define_flag planet : String, required: true so the error Flag required: --planet is correct. It should not be Argument required: planet since a flag is not an argument.

Currently, if you don't pass an argument it just shows a stack trace instead of Argument required: planet So I opened an issue here: https://github.com/jwaldrip/admiral.cr/issues/15

ravern commented 5 years ago

I personally would expect the following to be printed if a required argument is not provided.

I mean here that Argument required: planet should be printed if a required argument is not provided.

But yes, we are on the same page that the stack trace should not be the error when a required argument is not provided.