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

Add --install-dependencies option for phoenix.new #1247

Closed

Conversation

drewblas
Copy link

@drewblas drewblas commented Oct 3, 2015

Allows for programmatic running of phoenix.new without a shell prompt.

I did try adding a test, but the mix_shell message still throws an error because of some other input interaction. I think maybe this tiny flag doesn't need a test?

  test "new with --install-dependencies option" do
    in_tmp "new with --install-dependencies option" do
      Mix.Tasks.Phoenix.New.run [@app_name]
      refute_received {:mix_shell, :yes?, ["\nFetch and install dependencies?"]}
    end
  end
1) test new with --install-dependencies option (Mix.Tasks.Phoenix.NewTest)
     test/phoenix_new_test.exs:369
     ** (RuntimeError) no shell process input given for yes?/1
     stacktrace:
       (mix) lib/mix/shell/process.ex:155: Mix.Shell.Process.yes?/1
       (mix) lib/mix/generator.ex:20: Mix.Generator.create_file/3
       (phoenix_new) lib/phoenix_new.ex:505: anonymous fn/5 in Mix.Tasks.Phoenix.New.copy_from/3
       (elixir) lib/enum.ex:1385: Enum."-reduce/3-lists^foldl/2-0-"/3
       (phoenix_new) lib/phoenix_new.ex:494: Mix.Tasks.Phoenix.New.copy_from/3
       (phoenix_new) lib/phoenix_new.ex:206: Mix.Tasks.Phoenix.New.run/4
       test/phoenix_new_test.exs:371

Allows for programmatic running of phoenix.new 
without a shell prompt
@chrismccord
Copy link
Member

Piping to yes should accommodate this, right?

@drewblas
Copy link
Author

drewblas commented Oct 3, 2015

That's true in a general sense. Although most command line tools provide the option as well so that you don't need to "hack" around it with | yes.

So this brings it to be similar to other tools, like with mix archive.install: https://github.com/elixir-lang/elixir/blob/master/lib/mix/lib/mix/tasks/archive.install.ex#L27

Also, piping to | yes means answering yes to everything. But the user may want yes for some answers and no for others (like no to overwriting a file). Currently I think there's just the one phoenix prompt, but that could easily change (especially if a user has extra plugins that might start modifying how phoenix generates files).

@chrismccord
Copy link
Member

May I ask your usecase for programmatically calling the generators? I'm hesitant adding options around this because they could change in the future and I haven't see a usecase yet that requires them

@josevalim
Copy link
Member

Btw, yes | is a bad idea for long running tasks because it makes your memory grow if the data is not being consumed. I agree with @chrismccord I would like to hear use cases though before we add more options.

@drewblas
Copy link
Author

drewblas commented Oct 5, 2015

I have two use cases:

  1. I built https://github.com/drewblas/phoenix_upgrades to be able to compare how the phoenix new generators / boilerplate changes over time (as a complement to Chris' upgrade instructions)
  2. I'm working to build a more advanced set of generators (ala Rails Wizards or Rails Apps Composer) that starts with the base-generated app and then adds more based on user selections.

Quick note: I don't think this is ready to merge yet. What I realized when doing the first app is that I actually want to be able to explicitly answer 'no', not 'yes'. So this PR needs to be expanded to support --[no-]install-dependencies. But I'd like to get your feedback on if you will allow this command option before I go through the work of adding even more. Either way, thanks so much!

@josevalim
Copy link
Member

I think this is a reasonable use case. Let's see what @chrismccord thinks too! Thanks for expanding @drewblas!

@chrismccord
Copy link
Member

I agree it's definitely reasonable for what you want to achieve. Thanks for elaborating. Proceed!

@chrismccord
Copy link
Member

@drewblas ping. How are we coming on this one?

@chrismccord
Copy link
Member

@drewblas I'm closing for now. Please ping if you're up for completing this and I'll reopen. Thanks!

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