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.

Sign up for GitHub

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

Create add and remove subcommands for brew bundle #832

Closed
wants to merge 4 commits into from
Closed

Create add and remove subcommands for brew bundle #832

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5789e98
create `add_command_spec.rb`
johndbritton Dec 1, 2020
4446c9b
rough pass at implementing `brew bundle add` subcommand
johndbritton Dec 1, 2020
477f555
linting errors
johndbritton Dec 15, 2020
78adf4d
alternative approach
johndbritton Dec 16, 2020
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
5 changes: 5 additions & 0 deletions cmd/bundle.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ def bundle
zap: args.zap?,
)
end
when "add"
Bundle::Commands::Add.run(
global: args.global?,
file: args.file
johndbritton marked this conversation as resolved.
Show resolved Hide resolved
)
johndbritton marked this conversation as resolved.
Show resolved Hide resolved
when "dump"
Bundle::Commands::Dump.run(
global: args.global?,
Expand Down
31 changes: 31 additions & 0 deletions lib/bundle/commands/add.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

module Bundle
module Commands
module Add
module_function

def run(*args, global: false, file: nil)
raise UsageError, "No arguments were specified!" if args.blank?

type = :brew # default to brew
name = args.first # only support one formula at a time for now
options = {} # we don't currently support passing options
Comment on lines +11 to +13
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we handle different types of formulae? Is there an existing pattern for how you'd like this handled?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to just support brew for now and people can extend it later if desired. As long as it's documented it can raise UsageError which will print the help text.


# read the relevant Brewfile
parsed_entries = Bundle::Dsl.new(Brewfile.read(global: global, file: file)).entries

# check each of the entries in the specified Brewfile
parsed_entries.each do |entry|
# raise an error if the entry already exists in the Brewfile
# this could also be a noop, or print a friendly message
raise RuntimeError if entry.name == name
Copy link
Member

Choose a reason for hiding this comment

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

Yup, friendly message would be nice.

end

# need some help / pointers here
# is it possible to use Bundle::Dsl to create an in memory representation
#of the brewfile that we read, add an entry and then dump that back to the file?
Copy link
Member Author

Choose a reason for hiding this comment

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

This is where I could use the most help

Copy link
Member

Choose a reason for hiding this comment

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

This is the existing dumper code for this:

homebrew-bundle/lib/bundle/brew_dumper.rb

Lines 18 to 39 in 034459d

def formulae
@formulae ||= begin
@formulae = formulae_info
sort!
end
end
def dump(describe: false, no_restart: false)
requested_formula = formulae.select do |f|
f[:installed_on_request?] || !f[:installed_as_dependency?]
end
requested_formula.map do |f|
brewline = ""
brewline += desc_comment(f[:desc]) if describe && f[:desc]
brewline += "brew \"#{f[:full_name]}\""
args = f[:args].map { |arg| "\"#{arg}\"" }.sort.join(", ")
brewline += ", args: [#{args}]" unless f[:args].empty?
brewline += ", restart_service: true" if !no_restart && BrewServices.started?(f[:full_name])
brewline += ", link: #{f[:link?]}" unless f[:link?].nil?
brewline
end.join("\n")
end

It's probably possible to do that but you may have more luck trying to inject your desired formulae state into the existing logic (overriding @formulae somehow).

end
end
end
end
45 changes: 45 additions & 0 deletions spec/bundle/commands/add_command_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

require "spec_helper"

describe Bundle::Commands::Add do
context "when a Brewfile is not found" do
it "raises an error" do
expect { described_class.run("wget") }.to raise_error(RuntimeError)
end
end

context "when a Brewfile is found" do
before do
allow_any_instance_of(Pathname).to receive(:read)
.and_return("brew 'openssl'")
end

context "when no arguments are passed" do
it "raises an error" do
expect { described_class.run }.to raise_error(UsageError)
end
end

context "the formula is not in the Brewfile" do
it "does not raise an error" do
expect { described_class.run("wget") }.to_not raise_error
end

it "adds the formula to the Brewfile" do
#TODO
end
end

context "the formula is in the Brewfile" do
before do
allow_any_instance_of(Pathname).to receive(:read)
.and_return("brew 'openssl'\nbrew 'wget'")
end

it "raises an error" do
expect { described_class.run("wget") }.to raise_error(RuntimeError)
end
end
end
end