Default Array of String includes the square bracket on 2.1.1 about optimist HOT 9 CLOSED

@Student any thoughts on this one?

from optimist.

Yikes. I will try to look over this tonight.

from optimist.

git checkout 42b6912
ruby duku.rb --suites --debug ERROR
{:suites=>["["AwSuites"]"], :debug=>"ERROR", :help=>false, :suites_given=>true, :debug_given=>true}

git checkout 1b132c1
ruby duku.rb --suites --debug ERROR
Error: option '--suites' needs a parameter.

I have to take off. I hope to have a writeup tonight.

from optimist.

The changes from #11 assumed that the parameter type is singular, not an array. If the default is an array of integers, for instance, there will be a failure saying that the option needs an integer (what it has at that point is an array). Since strings are hit with :to_s, the array gets the treatment.

I don't see any approach to doing this that I like. Probably the least disruptive thing is to view the situation as exceptional:

      vals["#{sym}_given".intern] = true # mark argument as specified on the commandline

      opts = @specs[sym]
      if params.empty? && opts[:type] != :flag
        if opts.has_key?(:default)
          vals[sym] = opts[:default]
          next
        else
          raise CommandlineError, "option '#{arg}' needs a parameter"
        end
      end

That is, set the #{sym}_given up top, then shortcut out if we have a non-flag with no parameter but a default given.

from optimist.

@Student thanks. I'll add this use case to the test suite and see if that code fixes it.

Still trying to figure out the organization to this test suite.

from optimist.

@Student I had thought this was a change you introduced. Just realized it was not one of your PRs. Thanks for the thoughts and help.

@hansuiloe Would you be ok with the results if the array was properly populated?

@Fryguy I'm thinking about rolling #11 back. Any thoughts? [see #42 first]

from optimist.

Glad to be of help. :)

from optimist.

@kbrock Thanks for addressing this. Although you can't really differentiate between 2 and 4 example below (2.1.2), I guess this is acceptable for now.

c:\>ruby duku.rb --debug ERROR
opts[:suites]: ["AwSuites"]
opts[:suites_given]:

c:\>ruby duku.rb --suites  --debug ERROR
opts[:suites]: ["AwSuites"]
opts[:suites_given]: true

c:\>ruby duku.rb --suites a, b c --debug ERROR
opts[:suites]: ["a,", "b", "c"]
opts[:suites_given]: true

c:\>ruby ruby.rb --suites AwSuites --debug ERROR
opts[:suites]: ["AwSuites"]
opts[:suites_given]: true

from optimist.

@hansuiloe Yes. That is actually per design. If you have a --process to process records, defaulting to all, but optionally pass a parameter to specify a different value. This could be done with 2 options, but then you have to do special processing when you receive a value option but not the flag processing.

I'm mixed on whether I should have merged this feature request.

I had a bug in my code (didn't have a default), but I wonder how hard it would have been to track down if I did have a default.

db_script accepts parameter --user that defaults to ENV["PGUSER"].

wrapper_script calls the db_script via:

user_name = ""
system("db_script --user #{user_name}")

Yes, it is a bug in wrapper_script, but if --user has a default, then it won't show up until later. Again, curious how hard this feature would make tracking this down.

The fix for me was to ensure I had a value for user_name in wrapper_script.

Are you good for now?

from optimist.