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

Make Cask commands fail early if a Cask is not found. #3146

Merged
merged 1 commit into from
Sep 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions Library/Homebrew/cask/lib/hbc/cask_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,22 @@ def load
end
end

class FromInstanceLoader
attr_reader :cask

def self.can_load?(ref)
ref.is_a?(Cask)
end

def initialize(cask)
@cask = cask
end

def load
cask
end
end

class NullLoader < FromPathLoader
def self.can_load?(*)
true
Expand Down Expand Up @@ -149,6 +165,7 @@ def self.load(ref)

def self.for(ref)
[
FromInstanceLoader,
FromURILoader,
FromTapLoader,
FromTapPathLoader,
Expand Down
47 changes: 19 additions & 28 deletions Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,41 +42,32 @@ def initialize(*args)
@args = process_arguments(*args)
end

def self.warn_unavailable_with_suggestion(cask_token, e)
exact_match, partial_matches = Search.search(cask_token)
error_message = e.message
if exact_match
error_message.concat(" Did you mean:\n#{exact_match}")
elsif !partial_matches.empty?
error_message.concat(" Did you mean one of:\n")
.concat(Formatter.columns(partial_matches.take(20)))
end
onoe error_message
end

private

def casks(alternative: -> { [] })
return to_enum(:casks, alternative: alternative) unless block_given?

count = 0

return @casks if defined?(@casks)
casks = args.empty? ? alternative.call : args
@casks = casks.map { |cask| CaskLoader.load(cask) }
rescue CaskUnavailableError => e
reason = [e.reason, suggestion_message(e.token)].join(" ")
raise e.class.new(e.token, reason)
end

def suggestion_message(cask_token)
exact_match, partial_matches = Search.search(cask_token)

casks.each do |cask_or_token|
begin
yield cask_or_token.respond_to?(:token) ? cask_or_token : CaskLoader.load(cask_or_token)
count += 1
rescue CaskUnavailableError => e
cask_token = cask_or_token
self.class.warn_unavailable_with_suggestion cask_token, e
rescue CaskError => e
onoe e.message
end
if exact_match.nil? && partial_matches.count == 1
exact_match = partial_matches.first
end

return :empty if casks.length.zero?
(count == casks.length) ? :complete : :incomplete
if exact_match
"Did you mean “#{exact_match}”?"
elsif !partial_matches.empty?
"Did you mean one of these?\n"
.concat(Formatter.columns(partial_matches.take(20)))
else
""
end
end
end
end
Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/cask/lib/hbc/cli/cat.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ def initialize(*)
end

def run
raise CaskError, "Cat incomplete." if cat_casks == :incomplete
end

def cat_casks
casks.each do |cask|
puts File.open(cask.sourcefile_path, &:read)
end
Expand Down
19 changes: 8 additions & 11 deletions Library/Homebrew/cask/lib/hbc/cli/edit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,18 @@ class Edit < AbstractCommand
def initialize(*)
super
raise CaskUnspecifiedError if args.empty?
raise ArgumentError, "Only one Cask can be created at a time." if args.count > 1
raise ArgumentError, "Only one Cask can be edited at a time." if args.count > 1
end

def run
cask_token = args.first
cask_path = begin
CaskLoader.load(cask_token).sourcefile_path
rescue CaskUnavailableError => e
reason = e.reason.empty? ? "" : "#{e.reason} "
reason.concat("Run #{Formatter.identifier("brew cask create #{e.token}")} to create a new Cask.")
raise e.class.new(e.token, reason)
end

odebug "Opening editor for Cask #{cask_token}"
cask = casks.first
cask_path = cask.sourcefile_path
odebug "Opening editor for Cask #{cask.token}"
exec_editor cask_path
rescue CaskUnavailableError => e
reason = e.reason.empty? ? "" : "#{e.reason} "
reason.concat("Run #{Formatter.identifier("brew cask create #{e.token}")} to create a new Cask.")
raise e.class.new(e.token, reason)
end

def self.help
Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/cask/lib/hbc/cli/fetch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ def initialize(*)
end

def run
raise CaskError, "Fetch incomplete." if fetch_casks == :incomplete
end

def fetch_casks
casks.each do |cask|
ohai "Downloading external files for Cask #{cask}"
downloaded_path = Download.new(cask, force: force?).perform
Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/cask/lib/hbc/cli/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ def initialize(*)
end

def run
raise CaskError, "Install incomplete." if install_casks == :incomplete
end

def install_casks
casks.each do |cask|
begin
Installer.new(cask, binaries: binaries?,
Expand Down
26 changes: 10 additions & 16 deletions Library/Homebrew/cask/lib/hbc/cli/internal_appcast_checkpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def run
if args.all? { |t| t =~ %r{^https?://} && t !~ /\.rb$/ }
self.class.appcask_checkpoint_for_url(args)
else
self.class.appcask_checkpoint(args, calculate?)
self.class.appcask_checkpoint(load_casks, calculate?)
end
end

Expand All @@ -23,33 +23,27 @@ def self.appcask_checkpoint_for_url(urls)
end
end

def self.appcask_checkpoint(cask_tokens, calculate)
count = 0

cask_tokens.each do |cask_token|
cask = CaskLoader.load(cask_token)

def self.appcask_checkpoint(casks, calculate)
casks.each do |cask|
if cask.appcast.nil?
opoo "Cask '#{cask}' is missing an `appcast` stanza."
else
if calculate
checkpoint = if calculate
result = cask.appcast.calculate_checkpoint

checkpoint = result[:checkpoint]
result[:checkpoint]
else
checkpoint = cask.appcast.checkpoint
cask.appcast.checkpoint
end

if checkpoint.nil?
if calculate && checkpoint.nil?
onoe "Could not retrieve `appcast` checkpoint for cask '#{cask}': #{result[:command_result].stderr}"
elsif casks.count > 1
puts "#{checkpoint} #{cask}"
else
puts((cask_tokens.count > 1) ? "#{checkpoint} #{cask}" : checkpoint)
count += 1
puts checkpoint
end
end
end

count == cask_tokens.count
end

def self.help
Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/cask/lib/hbc/cli/internal_dump.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ def initialize(*)
end

def run
raise CaskError, "Dump incomplete." if dump_casks == :incomplete
end

def dump_casks
casks.each(&:dumpcask)
end

Expand Down
6 changes: 0 additions & 6 deletions Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ def initialize(*)
end

def run
return unless print_stanzas == :incomplete
exit 1 if quiet?
raise CaskError, "Print incomplete."
end

def print_stanzas
if ARTIFACTS.include?(stanza)
artifact_name = stanza
@stanza = :artifacts
Expand Down
5 changes: 1 addition & 4 deletions Library/Homebrew/cask/lib/hbc/cli/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ class List < AbstractCommand
end)

def run
retval = args.any? ? list : list_installed
raise CaskError, "Listing incomplete." if retval == :incomplete
args.any? ? list : list_installed
end

def list
Expand Down Expand Up @@ -46,8 +45,6 @@ def list_installed
elsif !installed_casks.empty?
puts Formatter.columns(installed_casks.map(&:to_s))
end

installed_casks.empty? ? :empty : :complete
end

def self.format_versioned(cask)
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/cli/reinstall.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Hbc
class CLI
class Reinstall < Install
def install_casks
def run
casks.each do |cask|
Installer.new(cask, binaries: binaries?,
verbose: verbose?,
Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ def initialize(*)
end

def run
raise CaskError, "Uninstall incomplete." if uninstall_casks == :incomplete
end

def uninstall_casks
casks.each do |cask|
odebug "Uninstalling Cask #{cask}"

Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/cask/lib/hbc/cli/zap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ def initialize(*)
end

def run
raise CaskError, "Zap incomplete." if zap_casks == :incomplete
end

def zap_casks
casks.each do |cask|
odebug "Zapping Cask #{cask}"
Installer.new(cask, verbose: verbose?, force: force?).zap
Expand Down
4 changes: 3 additions & 1 deletion Library/Homebrew/test/cask/cli/audit_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
describe Hbc::CLI::Audit, :cask do
let(:cask) { double("cask", token: nil) }
let(:cask) { Hbc::Cask.new(nil) }

describe "selection of Casks to audit" do
it "audits all Casks if no tokens are given" do
expect(cask).to be_a Hbc::Cask

allow(Hbc).to receive(:all).and_return([cask, cask])

expect(Hbc::Auditor).to receive(:audit).twice.and_return(true)
Expand Down
3 changes: 1 addition & 2 deletions Library/Homebrew/test/cask/cli/cat_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@

it "raises an exception when the Cask does not exist" do
expect { Hbc::CLI::Cat.run("notacask") }
.to output(/is unavailable/).to_stderr
.and raise_error(Hbc::CaskError, "Cat incomplete.")
.to raise_error(Hbc::CaskUnavailableError, /is unavailable/)
end

describe "when no Cask is specified" do
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/cask/cli/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
it "raises an exception when more than one Cask is given" do
expect {
described_class.run("additional-cask", "another-cask")
}.to raise_error(/Only one Cask can be created at a time./)
}.to raise_error(/Only one Cask can be created at a time\./)
end

it "raises an exception when the Cask already exists" do
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/cask/cli/edit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
it "raises an error when given more than one argument" do
expect {
described_class.new("local-caffeine", "local-transmission")
}.to raise_error(/Only one Cask can be created at a time./)
}.to raise_error(/Only one Cask can be edited at a time\./)
end

it "raises an exception when the Cask doesnt exist" do
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/cask/cli/fetch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
it "properly handles Casks that are not present" do
expect {
Hbc::CLI::Fetch.run("notacask")
}.to raise_error(Hbc::CaskError, "Fetch incomplete.")
}.to raise_error(Hbc::CaskUnavailableError)
end

describe "when no Cask is specified" do
Expand Down
18 changes: 5 additions & 13 deletions Library/Homebrew/test/cask/cli/install_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,27 +56,19 @@
it "properly handles Casks that are not present" do
expect {
Hbc::CLI::Install.run("notacask")
}.to raise_error(Hbc::CaskError, "Install incomplete.")
}.to raise_error(Hbc::CaskUnavailableError)
end

it "returns a suggestion for a misspelled Cask" do
expect {
begin
Hbc::CLI::Install.run("localcaffeine")
rescue Hbc::CaskError
nil
end
}.to output(/Cask 'localcaffeine' is unavailable: No Cask with this name exists\. Did you mean:\nlocal-caffeine/).to_stderr
Hbc::CLI::Install.run("localcaffeine")
}.to raise_error(Hbc::CaskUnavailableError, /Cask 'localcaffeine' is unavailable: No Cask with this name exists\. Did you mean “local-caffeine”?/)
end

it "returns multiple suggestions for a Cask fragment" do
expect {
begin
Hbc::CLI::Install.run("local-caf")
rescue Hbc::CaskError
nil
end
}.to output(/Cask 'local-caf' is unavailable: No Cask with this name exists\. Did you mean one of:\nlocal-caffeine/).to_stderr
Hbc::CLI::Install.run("local")
}.to raise_error(Hbc::CaskUnavailableError, /Cask 'local' is unavailable: No Cask with this name exists\. Did you mean one of these\?\nlocal-caffeine\nlocal-transmission/)
end

describe "when no Cask is specified" do
Expand Down
9 changes: 3 additions & 6 deletions Library/Homebrew/test/cask/cli/uninstall_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@

it "shows an error when a bad Cask is provided" do
expect { Hbc::CLI::Uninstall.run("notacask") }
.to output(/is unavailable/).to_stderr
.and raise_error(Hbc::CaskError, "Uninstall incomplete.")
.to raise_error(Hbc::CaskUnavailableError, /is unavailable/)
end

it "shows an error when a Cask is provided that's not installed" do
expect { Hbc::CLI::Uninstall.run("local-caffeine") }
.to output(/is not installed/).to_stderr
.and raise_error(Hbc::CaskError, "Uninstall incomplete.")
.to raise_error(Hbc::CaskNotInstalledError, /is not installed/)
end

it "tries anyway on a non-present Cask when --force is given" do
Expand Down Expand Up @@ -76,8 +74,7 @@
Hbc.appdir.join("MyFancyApp.app").rmtree

expect { Hbc::CLI::Uninstall.run("with-uninstall-script-app") }
.to output(/does not exist/).to_stderr
.and raise_error(Hbc::CaskError, "Uninstall incomplete.")
.to raise_error(Hbc::CaskError, /uninstall script .* does not exist/)

expect(cask).to be_installed

Expand Down
3 changes: 1 addition & 2 deletions Library/Homebrew/test/cask/cli/zap_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
describe Hbc::CLI::Zap, :cask do
it "shows an error when a bad Cask is provided" do
expect { Hbc::CLI::Zap.run("notacask") }
.to output(/is unavailable/).to_stderr
.and raise_error(Hbc::CaskError, "Zap incomplete.")
.to raise_error(Hbc::CaskUnavailableError, /is unavailable/)
end

it "can zap and unlink multiple Casks at once" do
Expand Down