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

Added a liftoffrc config flag to only prompt on attributes with no specified default value #127

Closed
wants to merge 3 commits into from

Conversation

jpsim
Copy link
Contributor

@jpsim jpsim commented Mar 20, 2014

This is especially useful when used with CLI config overrides (#126).

As an example, suppose you have the following .liftoffrc:

company: ACME Inc.
company_identifier: com.example
author: John Doe
prefix: ACM
strict_prompts: true

and you run liftoff, you'l only be prompted for the project name before the project gets created.

I have another incoming PR that combines this with #126 by adding a CLI option --strict-prompts, making this infinitely more useful because it allows doing this:
liftoff -n ProjName --strict-prompts
and you don't get prompted at all.

@@ -1,6 +1,6 @@
module Liftoff
class ProjectConfiguration
attr_accessor :project_name, :company, :prefix, :configure_git, :warnings_as_errors, :install_todo_script, :enable_static_analyzer, :indentation_level, :warnings, :application_target_groups, :unit_test_target_groups, :use_cocoapods
attr_accessor :project_name, :company, :prefix, :configure_git, :warnings_as_errors, :install_todo_script, :enable_static_analyzer, :indentation_level, :warnings, :application_target_groups, :unit_test_target_groups, :use_cocoapods, :strict_prompts

Choose a reason for hiding this comment

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

Line is too long. [252/80]

@gfontenot
Copy link
Member

Can you rebase #128 onto master and include these changes, and we'll treat it as one PR?

@jpsim
Copy link
Contributor Author

jpsim commented Mar 27, 2014

Combined with #128 and rebased against master.

@@ -30,6 +30,10 @@ def global_options
exit
end

opts.on('--[no-]strict-prompts', 'Enable/Disable strict prompts') do |strict_prompts|
Copy link
Member

Choose a reason for hiding this comment

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

Does this work? I thought we couldn't have hyphens in the flag? Or was that just for flags that take a value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it works for flags that already have a modifier. That's why it works for --[no-]strict-prompts but not for --indentation-level. optparse has some pretty eccentric parsing logic.

Copy link
Member

Choose a reason for hiding this comment

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

God that's stupid.

rescue EOFError
puts
fetch_option_for(attribute, prompt)
rescue Interrupt
exit 1
end

def prompt_for_default?(default)
!default || !@configuration.strict_prompts
Copy link
Member

Choose a reason for hiding this comment

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

This still bugs me, sorry. Talked it over with some ruby folks, and we kind of settled on flipping this conditional and using unless on line 18:

def skip_prompt?(default)
  default && @configuration.strict_prompts
end

My brain doesn't do as many backflips with it written out this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

@gfontenot
Copy link
Member

Fantastic. Rebased/Squashed/Merged as c6a05f8. Thanks!

@gfontenot gfontenot closed this Mar 27, 2014
@jpsim jpsim deleted the jp-strict-prompts branch March 27, 2014 20:40
@jpsim
Copy link
Contributor Author

jpsim commented Mar 27, 2014

Yay!

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