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

Adding leading newline to gem append_file string #3751

Closed
wants to merge 1 commit into from

Conversation

johnmdonahue
Copy link
Contributor

Issue

The gem action appends to the Gemfile without inserting a leading newline which can break installers.

Steps to Reproduce

Create a TestApp:

$ rails new TestApp
$ cd TestApp/

Edit the Gemfile to add rails_admin to the bottom with no trailing line break:

.
.
gem 'rails_admin', :git => 'git://github.com/sferik/rails_admin.git'

Bundle and run rails_admin installer:

$ bundle install
$ rails g rails_admin:install

Results in the following edited Gemfile:

.
.
gem 'rails_admin', :git => 'git://github.com/sferik/rails_admin.git'gem "devise"

Which fails on generate devise:install:

generate  devise:install
/Users/username/.rvm/gems/ruby-1.9.3-head/gems/bundler-1.0.21/lib/bundler/dsl.rb:7:in `instance_eval': /Users/username/TestApp/Gemfile:38: syntax error, unexpected tIDENTIFIER, expecting $end (SyntaxError)
....com/sferik/rails_admin.git'gem "devise"

Solution

This appears to be a problem with the gem generator action, where
append_file is called with a trailing newline but not a leading one.

in_root do
  str = "gem #{parts.join(", ")}\n" # Problem here
  str = "  " + str if @in_group
  append_file "Gemfile", str, :verbose => false
end

Without getting tricky, it seems like it might make sense to add a leading newline here:

in_root do
  str = "\ngem #{parts.join(", ")}\n"
  str = "  " + str if @in_group
  append_file "Gemfile", str, :verbose => false
end

Thoughts?

@josevalim
Copy link
Contributor

Looks good. We just need a test, thanks!

@bbenezech
Copy link

👍

josevalim added a commit that referenced this pull request Nov 25, 2011
@josevalim josevalim closed this Nov 25, 2011
@josevalim
Copy link
Contributor

Sorry, just now I have realized that the authors of this pull request and #3752 (that got merged) were different. @ganeshkumar, the next time please include the original patch in your pull request so both authors gets commits in.

@johnmdonahue
Copy link
Contributor Author

The test and fix on #3752 render a group block without indentation.

group :development, :test do

gem "rspec-rails"
end

I have updated my branch to handle gem groups as well as the leading newline complete with a test.

group :development, :test do

  gem "rspec-rails"
end

@josevalim josevalim mentioned this pull request Nov 25, 2011
@sukeerthiadiga
Copy link
Contributor

+1

@johnmdonahue
Copy link
Contributor Author

I guess my question/concern here is that the modified test does not check for leading newline:

/^gem "rspec-rails"$/

Also the output for gem groups now has no indentation.

A better possible solution here that maintains existing linebreaks and indentation as well as actually testing for leading newline.

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.

None yet

4 participants