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

A gem action #1066

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 180 additions & 0 deletions lib/suspenders/actions.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "strscan"

module Suspenders
module Actions
def replace_in_file(relative_path, find, replace)
Expand Down Expand Up @@ -31,6 +33,10 @@ def expand_json(file, data)
action ExpandJson.new(destination_root, file, data)
end

def gem(name, version = nil, **options)
action Gem.new(destination_root, name, version, **options)
end

class ExpandJson
def initialize(destination_root, file, data)
@destination_root = destination_root
Expand Down Expand Up @@ -81,5 +87,179 @@ def hash_unmerge(hash, subhash)
end
end
end

class Gem
def initialize(destination_root, name, version, **options)
@destination_root = destination_root
@name = name
@version = version
@groups = Array(options.delete(:group))
@options = options
end

def invoke!
existing_gemfile = IO.read(gemfile_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we need to pass existing_gemfile around to all the methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to avoid doing IO in the initializer. But maybe it's fine so long as I'm not, like, writing to disc in the initializer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah it's a pretty good reason and it's already this way so your choice on the matter. It would certainly help the arity of the private methods stay to <=2 but at the end of the day it's minor.

Copy link
Contributor

@thiagoa thiagoa Aug 13, 2021

Choose a reason for hiding this comment

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

These are all good reasons. One option is to pass the IO object to the initializer, but not very practical. Maybe we could do something like this:

def existing_gemfile
  @existing_gemfile ||= IO.read(gemfile_path)
end

Which incurs the IO cost only once and avoids duplication (at the expense of "where does this come from?").


if groups.present?
positioning = gem_group(existing_gemfile)

if positioning
insert_gemline(existing_gemfile, indentation: 2, positioning: positioning)
else
positioning = final_group(existing_gemfile)

if positioning
insert_gemline(existing_gemfile, indentation: 2, positioning: positioning)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to also generate the group block here and below. Whoops!

else
insert_gemline(existing_gemfile, indentation: 2, positioning: [0, existing_gemfile.bytes.size])
end
end
else
insert_gemline(existing_gemfile, indentation: 0, positioning: [0, existing_gemfile.bytes.size])
end
Comment on lines +103 to +119
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a deep enough nested conditional block it might benefit from some more method extractions for readability

end

def revoke!
existing_gemfile = IO.read(gemfile_path)

if groups.present?
positioning = gem_group(existing_gemfile)

if positioning
remove_gemline(existing_gemfile, indentation: 2, positioning: positioning)
end
else
remove_gemline(existing_gemfile, indentation: 0, positioning: [0, existing_gemfile.bytes.size])
end
end

private

attr_reader :destination_root, :name, :version, :groups, :options

def gemfile_path
File.join(destination_root, "Gemfile")
end

def gem_group(gemfile)
groups_re = groups.map {|group| ":#{group}" }.join("[, ]+")
data = /^group[ (]+#{groups_re}[ )]+do.+?^end/mo.match(gemfile)
Copy link
Contributor

Choose a reason for hiding this comment

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

Small thing: This works for the case when the groups are in the order specified (ie: :test, :development vs. :development, :test). Don't really think it's worth addressing since I don't know a simple way to do it with regex and it's simple enough to just make sure we are specifying the correct order in all of our calls to gem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right ... we could explode all the permutations with an | but it really doesn't seem worth it for just Suspenders.


if data
[data.begin(0), data[0].bytes.size]
end
end

def final_group(gemfile)
scanner = StringScanner.new(gemfile)
result = nil

while scanner.scan_until(/^group.+?do.+?^end+/m)
if scanner.matched?
result = [scanner.pos, scanner.matched.bytes.size]
end
end

result
end

def insert_gemline(gemfile, indentation:, positioning:)
position = subsequent_gemline(gemfile, indentation: indentation, positioning: positioning)

if position
insert_gemline_at(gemfile, positioning[0] + position, indentation: indentation)
else
final_positioning = final_line(gemfile, indentation: indentation, positioning: positioning)

if final_positioning
insert_gemline_at(gemfile, positioning[0] + final_positioning[1], indentation: indentation)
else
insert_gemline_at(gemfile, positioning[0] + positioning[1], indentation: indentation)
end
end
end

def remove_gemline(old_content, indentation:, positioning:)
group_without_gem = old_content.byteslice(positioning[0], positioning[1]).
sub(/^#{" " * indentation}gem[ (]+['"]#{name}.*\n/, "")
new_content = old_content.byteslice(0, positioning[0]) +
group_without_gem +
old_content.byteslice((positioning[0] + positioning[1])..)

announce
IO.write(gemfile_path, new_content)
end

def subsequent_gemline(gemfile, indentation:, positioning:)
scanner = StringScanner.new(
gemfile.byteslice(positioning[0], positioning[1]),
)

while scanner.scan_until(/^#{" " * indentation}gem[ (]+['"]([^'"]+)/)
if name < scanner.captures[0]
return scanner.pos - scanner.matched.bytes.size
end
end

nil
end

def insert_gemline_at(old_content, position, indentation:)
announce
IO.write(
gemfile_path,
new_content(old_content, position: position, indentation: indentation),
)
end

def new_content(old_content, position:, indentation:)
ensure_newline((old_content.byteslice(0, position) || "")) +
(" " *indentation) +
gemline +
ensure_newline((old_content.byteslice(position..) || ""))
end

def final_line(gemfile, indentation:, positioning:)
scanner = StringScanner.new(
gemfile.byteslice(positioning[0], positioning[1]),
)
result = nil

while scanner.scan_until(/#{" " *indentation}gem[ (]+['"][^'"]+['"]/)
result = [scanner.pos, scanner.matched.bytes.size]
end

result
end

def ensure_newline(str)
"#{str.chomp}\n"
end

def gemline
parts = [%{"#{name}"}, version]
mike-burns marked this conversation as resolved.
Show resolved Hide resolved

options&.each_pair do |k,v|
parts << %{#{k}: "#{v}"}
end

"gem #{parts.compact.join(", ")}\n"
end

def announce
message = "#{name}"

if version
message << " (#{version})"
end

if options[:git]
message = options[:git]
end

Thor::Base.shell.new.say_status :gemfile, message
end

end
end
end
5 changes: 4 additions & 1 deletion spec/features/ci_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
end

it "removes Circle and SimpleCov" do
with_app { destroy("suspenders:ci") }
with_app do
generate("suspenders:ci")
destroy("suspenders:ci")
end

expect("circle.yml").not_to exist_as_a_file
expect("test/test_helper.rb").not_to match_contents(/SimpleCov/)
Expand Down
5 changes: 4 additions & 1 deletion spec/features/static_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
end

it "removes the gem and pages directory" do
with_app { destroy("suspenders:static") }
with_app do
generate("suspenders:static")
destroy("suspenders:static")
end

expect("app/views/pages/.keep").not_to exist_as_a_file
expect("Gemfile").not_to match_contents(/high_voltage/)
Expand Down