Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow defining options starting with capital letters #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

porras
Copy link
Contributor

@porras porras commented Oct 2, 2018

Having options and flags starting with a capital letter is relatively frequent (optarg supports it for the convention of using it to negate the lowercase version). Currently, defining it independently doesn't work (as it tries to dinamically generate an invalid method name):

require "optarg"

class Parser < Optarg::Model
  bool ["-V", "--version"]
end
Error in opts.cr:4: expanding macro

  bool ["-V", "--version"]
  ^

in macro 'bool' /home/sergio/Code/crul/lib/optarg/src/lib/model/dsl/bool.cr:4, line 1:

>  1.       __define_static_value :option, :predicate, ::Optarg::Definitions::BoolOption, ["-V", "--version"], nil, nil do  |klass|
   2.         option = klass.new(["-V", "--version"], metadata: nil, stop: nil, default: nil)
   3.         @@__klass.definitions << option
   4.         
   5.         
   6.       end
   7.     

macro didn't expand to a valid program, it expanded to:

================================================================================
--------------------------------------------------------------------------------
   1.       
   2. 
   3.       
   4.         
   5.       
   6. 
   7.       
   8.         
   9.           
  10.             # Returns the -V option value.
  11.             #
  12.             # This method is automatically defined by the optarg library.
  13.             def V?
  14.               !!self[::Optarg::Definitions::BoolOption::Typed::Type]["-V"]?
  15.             end
  16.           
  17.                                                          
  18.                                                          
  19.                                                          
  20.                                                          
  21.                                                          
  22.                                                          
  23.             # Returns the -V option value.               
  24.             #                                            
  25.             # This method is automatically defined by the optarg library.
  26.             def version?                                 
  27.               !!self[::Optarg::Definitions::BoolOption::Typed::Type]["-V"]?
  28.             end
  29.           
  30.         
  31.         
  32.         
  33.       
  34. 
  35.       ::Parser.__with_self(::Optarg::Definitions::BoolOption) do |klass|
  36.   option = klass.new(["-V", "--version"], metadata: nil, stop: nil, default: nil)
  37.   @@__klass.definitions << option
  38. end
  39.     
--------------------------------------------------------------------------------
Syntax error in expanded macro: __define_static_value:13: unexpected token: NEWLINE

            def V?
                  ^

================================================================================

In the implementation, I just skip the generation of that method, following the same rationale as when reserved words are used (--class) that the value is still available through the hash lookup, or if we defined any synonym. The regular expression could be expanded to catch other valid options that generate invalid method names (numbers come to mind), but I kept it as simple as possible for my use case. Feel free to expand it (or ask me to do it).

@mosop mosop self-assigned this Feb 7, 2019
@mosop mosop self-requested a review February 7, 2019 08:49
@m-o-e
Copy link

m-o-e commented Dec 2, 2019

+1

I also bumped into this, can it be fixed please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants