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

Introduces convenience macros for int32 and float64 for task options #3

Merged
merged 7 commits into from
Sep 28, 2021

Conversation

mwlang
Copy link
Contributor

@mwlang mwlang commented Sep 26, 2021

Similar in concept to the switch macro for generating Bool options, int32 and float64 introduces convenience macros for defining options that are expected to result in Int32 or Float64 input values.

Example:

class TaskWithInt32Flags < LuckyTask::Task
  summary "This is a task with int32 flags"

  int32 :zero, "going to zero in a hurry"
  int32 :uno, description: "defaults to one", shortcut: "-u", default: 1

  def call
    self
  end
end

class TaskWithFloat64Flags < LuckyTask::Task
  summary "This is a task with float64 flags"

  float64 :zero, "going to zero in a hurry"
  float64 :uno, description: "defaults to one", shortcut: "-u", default: 1
  float64 :pi, description: "defaults to PI", shortcut: "-p", default: 3.14

  def call
    self
  end
end
  • int32 is implemented with flexibility to supply values in the form of 1000, 1_000, +1000, -1000
  • float64 is implemented with flexibility to supply values in the form of 1, 1.0, -1.0, +1.0, 1_000.0, 10_000.000_001

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this! Overall I think it's fine... The only thing I'm a little hesitant on is the use of comma as a number separator (1,000). Since not all people use that character as a number separator, it starts to creep in to the gray area of "do we need configs for people in different countries?", etc... I think if we stick to just what Crystal uses to format Int and Float, then it's a bit more universal.

What do you think?

@mwlang
Copy link
Contributor Author

mwlang commented Sep 28, 2021

I debated that one myself as well. I know many countries use . instead of , for the decimal group delimiter.

Let's take it out to comply with what Crystal does and leave it at that. I'll update the PR.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 Thanks for putting this together!

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.

2 participants