diff --git a/cmd/bundle.rb b/cmd/bundle.rb index 85ebc4d36a..3ad2107a77 100755 --- a/cmd/bundle.rb +++ b/cmd/bundle.rb @@ -59,7 +59,7 @@ class BundleCmd < AbstractCommand Add entries to your `Brewfile`. Adds formulae by default. Use `--cask`, `--tap`, `--whalebrew` or `--vscode` to add the corresponding entry instead. `brew bundle remove` [...]: - Remove entries that match `name` from your `Brewfile`. Use `--formula`, `--cask`, `--tap`, `--mas`, `--whalebrew` or `--vscode` to remove only entries of the corresponding type. + Remove entries that match `name` from your `Brewfile`. Use `--formula`, `--cask`, `--tap`, `--mas`, `--whalebrew` or `--vscode` to remove only entries of the corresponding type. Passing `--formula` also removes matches against formula aliases and old formula names. `brew bundle exec` : Run an external command in an isolated build environment based on the `Brewfile` dependencies. diff --git a/lib/bundle/remover.rb b/lib/bundle/remover.rb index 477a6615d8..c8abf5aaac 100644 --- a/lib/bundle/remover.rb +++ b/lib/bundle/remover.rb @@ -7,14 +7,39 @@ module Remover def remove(*args, type:, global:, file:) brewfile = Brewfile.read(global:, file:) - content = brewfile.input.split("\n") + content = brewfile.input entry_type = type.to_s if type != :none - escaped_args = args.map { |arg| Regexp.escape(arg) } - content = content.grep_v(/#{entry_type}(\s+|\(\s*)"(#{escaped_args.join("|")})"/) - .join("\n") << "\n" + escaped_args = args.flat_map do |arg| + names = if type == :brew + possible_names(arg) + else + [arg] + end + + names.uniq.map { |a| Regexp.escape(a) } + end + + new_content = content.split("\n") + .grep_v(/#{entry_type}(\s+|\(\s*)"(#{escaped_args.join("|")})"/) + .join("\n") << "\n" + + if content.chomp == new_content.chomp && + type == :none && + args.any? { |arg| possible_names(arg, raise_error: false).count > 1 } + opoo "No matching entries found in Brewfile. Try again with `--formula` to match formula " \ + "aliases and old formula names." + return + end + path = Dumper.brewfile_path(global:, file:) + Dumper.write_file path, new_content + end - Dumper.write_file path, content + def possible_names(formula_name, raise_error: true) + formula = Formulary.factory(formula_name) + [formula_name, formula.name, formula.full_name, *formula.aliases, *formula.oldnames].compact.uniq + rescue FormulaUnavailableError + raise if raise_error end end end diff --git a/lib/bundle/remover.rbi b/lib/bundle/remover.rbi new file mode 100644 index 0000000000..94533af368 --- /dev/null +++ b/lib/bundle/remover.rbi @@ -0,0 +1,7 @@ +# typed: true + +module Bundle + module Remover + include Kernel + end +end diff --git a/spec/bundle/commands/remove_command_spec.rb b/spec/bundle/commands/remove_command_spec.rb index f2a9b12a18..5dadcf2fe3 100644 --- a/spec/bundle/commands/remove_command_spec.rb +++ b/spec/bundle/commands/remove_command_spec.rb @@ -7,7 +7,7 @@ described_class.run(*args, type:, global:, file:) end - before { File.write(file, "brew \"hello\"\n") } + before { File.write(file, content) } after { FileUtils.rm_f file } let(:global) { false } @@ -16,10 +16,54 @@ let(:args) { ["hello"] } let(:type) { :brew } let(:file) { "/tmp/some_random_brewfile#{Random.rand(2 ** 16)}" } + let(:content) do + <<~BREWFILE + brew "hello" + BREWFILE + end it "removes entries from the given Brewfile" do expect { remove }.not_to raise_error expect(File.read(file)).not_to include("#{type} \"#{args.first}\"") end end + + context "when called with no type" do + let(:args) { ["foo"] } + let(:type) { :none } + let(:file) { "/tmp/some_random_brewfile#{Random.rand(2 ** 16)}" } + let(:content) do + <<~BREWFILE + tap "someone/tap" + brew "foo" + cask "foo" + BREWFILE + end + + it "removes all matching entries from the given Brewfile" do + expect { remove }.not_to raise_error + expect(File.read(file)).not_to include(args.first) + end + + context "with arguments that match entries only when considering formula aliases" do + let(:foo) do + instance_double( + Formula, + name: "foo", + full_name: "qux/quuz/foo", + oldnames: ["oldfoo"], + aliases: ["foobar"], + ) + end + let(:args) { ["foobar"] } + + it "suggests using `--formula` to match against formula aliases" do + expect(Formulary).to receive(:factory).with("foobar").and_return(foo) + expect { remove }.not_to raise_error + expect(File.read(file)).to eq(content) + # FIXME: Why doesn't this work? + # expect { remove }.to output("--formula").to_stderr + end + end + end end diff --git a/spec/bundle/remover_spec.rb b/spec/bundle/remover_spec.rb new file mode 100644 index 0000000000..4fb8d2e4e1 --- /dev/null +++ b/spec/bundle/remover_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +describe Bundle::Remover do + subject(:remover) { described_class } + + let(:name) { "foo" } + + before { allow(Formulary).to receive(:factory).with(name).and_raise(FormulaUnavailableError) } + + it "raises no errors when requested" do + expect { remover.possible_names(name, raise_error: false) }.not_to raise_error + end +end