From 30c313b7b9252019bef5134ecf6cb3fdd14197ea Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 7 Apr 2020 08:32:30 +0200 Subject: [PATCH] Refactor cask command parsing logic. --- Library/Homebrew/cask/cmd.rb | 188 ++++++++---------- Library/Homebrew/cask/cmd/abstract_command.rb | 2 +- .../cask/cmd/abstract_internal_command.rb | 2 +- Library/Homebrew/cask/cmd/help.rb | 42 ++++ Library/Homebrew/cask/cmd/internal_help.rb | 8 +- Library/Homebrew/test/cask/cmd_spec.rb | 38 ++-- 6 files changed, 152 insertions(+), 128 deletions(-) create mode 100644 Library/Homebrew/cask/cmd/help.rb diff --git a/Library/Homebrew/cask/cmd.rb b/Library/Homebrew/cask/cmd.rb index 039548e896..f81026ffa9 100644 --- a/Library/Homebrew/cask/cmd.rb +++ b/Library/Homebrew/cask/cmd.rb @@ -17,6 +17,7 @@ require "cask/cmd/create" require "cask/cmd/doctor" require "cask/cmd/edit" require "cask/cmd/fetch" +require "cask/cmd/help" require "cask/cmd/home" require "cask/cmd/info" require "cask/cmd/install" @@ -37,9 +38,7 @@ module Cask ALIASES = { "ls" => "list", "homepage" => "home", - "-S" => "search", # verb starting with "-" is questionable - "up" => "update", - "instal" => "install", # gem does the same + "instal" => "install", # gem does the same "uninstal" => "uninstall", "rm" => "uninstall", "remove" => "uninstall", @@ -86,38 +85,7 @@ module Cask def self.lookup_command(command_name) @lookup ||= Hash[commands.zip(command_classes)] command_name = ALIASES.fetch(command_name, command_name) - @lookup.fetch(command_name, command_name) - end - - def self.run_command(command, *args) - return command.run(*args) if command.respond_to?(:run) - - tap_cmd_directories = Tap.cmd_directories - - path = PATH.new(tap_cmd_directories, ENV["HOMEBREW_PATH"]) - - external_ruby_cmd = tap_cmd_directories.map { |d| d/"brewcask-#{command}.rb" } - .find(&:file?) - external_ruby_cmd ||= which("brewcask-#{command}.rb", path) - - if external_ruby_cmd - require external_ruby_cmd - - klass = begin - const_get(command.to_s.capitalize.to_sym) - rescue NameError - # External command is a stand-alone Ruby script. - return - end - - return klass.run(*args) - end - - if external_command = which("brewcask-#{command}", path) - exec external_command, *ARGV[1..-1] - end - - NullCommand.new(command, *args).run + @lookup.fetch(command_name, nil) end def self.run(*args) @@ -128,35 +96,59 @@ module Cask @args = process_options(*args) end - def detect_command_and_arguments(*args) - command = args.find do |arg| - if self.class.commands.include?(arg) - true - else - break unless arg.start_with?("-") + def find_external_command(command) + @tap_cmd_directories ||= Tap.cmd_directories + @path ||= PATH.new(@tap_cmd_directories, ENV["HOMEBREW_PATH"]) + + external_ruby_cmd = @tap_cmd_directories.map { |d| d/"brewcask-#{command}.rb" } + .find(&:file?) + external_ruby_cmd ||= which("brewcask-#{command}.rb", @path) + + if external_ruby_cmd + ExternalRubyCommand.new(command, external_ruby_cmd) + elsif external_command = which("brewcask-#{command}", @path) + ExternalCommand.new(external_command) + end + end + + def detect_internal_command(*args) + args.each_with_index do |arg, i| + if command = self.class.lookup_command(arg) + args.delete_at(i) + return [command, args] + elsif !arg.start_with?("-") + break end end - if index = args.index(command) - args.delete_at(index) + nil + end + + def detect_external_command(*args) + args.each_with_index do |arg, i| + if command = find_external_command(arg) + args.delete_at(i) + return [command, args] + elsif !arg.start_with?("-") + break + end end - [*command, *args] + nil end def run - command_name, *args = detect_command_and_arguments(*@args) - command = if help? - args.unshift(command_name) unless command_name.nil? - "help" - else - self.class.lookup_command(command_name) - end - MacOS.full_version = ENV["MACOS_VERSION"] unless ENV["MACOS_VERSION"].nil? - Tap.default_cask_tap.install unless Tap.default_cask_tap.installed? - self.class.run_command(command, *args) + + args = @args.dup + command, args = detect_internal_command(*args) || detect_external_command(*args) || [NullCommand.new, args] + + if help? + puts command.help + else + command.run(*args) + end rescue CaskError, MethodDeprecatedError, ArgumentError, OptionParser::InvalidOption => e onoe e.message $stderr.puts e.backtrace if ARGV.debug? @@ -190,16 +182,18 @@ module Cask def process_options(*args) exclude_regex = /^\-\-#{Regexp.union(*Config::DEFAULT_DIRS.keys.map(&Regexp.public_method(:escape)))}=/ - all_args = Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) - .reject { |arg| arg.match?(exclude_regex) } + args - non_options = [] - if idx = all_args.index("--") - non_options += all_args.drop(idx) - all_args = all_args.first(idx) + if idx = args.index("--") + non_options += args.drop(idx) + args = args.first(idx) end + cask_opts = Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) + .reject { |arg| arg.match?(exclude_regex) } + + all_args = cask_opts + args + remaining = all_args.select do |arg| !process_arguments([arg]).empty? rescue OptionParser::InvalidOption, OptionParser::MissingArgument, OptionParser::AmbiguousOption @@ -209,53 +203,45 @@ module Cask remaining + non_options end - class NullCommand - def initialize(command, *args) - @command = command - @args = args + class ExternalRubyCommand + def initialize(command, path) + @command_name = command.to_s.capitalize.to_sym + @path = path + end + + def run(*args) + require @path + + klass = begin + Cmd.const_get(@command_name) + rescue NameError + return + end + + klass.run(*args) + end + end + + class ExternalCommand + def initialize(path) + @path = path end def run(*) - purpose - usage + exec @path, *ARGV[1..-1] + end + end - return if @command.nil? - - if @command == "help" - return if @args.empty? - - raise ArgumentError, "help does not take arguments." if @args.length + class NullCommand + def run(*args) + if args.empty? + ofail "No subcommand given.\n" + else + ofail "Unknown subcommand: #{args.first}" end - raise ArgumentError, "Unknown Cask command: #{@command}" - end - - def purpose - puts <<~EOS - Homebrew Cask provides a friendly CLI workflow for the administration - of macOS applications distributed as binaries. - - EOS - end - - def usage - max_command_len = Cmd.commands.map(&:length).max - - puts "Commands:\n\n" - Cmd.command_classes.each do |klass| - next unless klass.visible - - puts " #{klass.command_name.ljust(max_command_len)} #{_help_for(klass)}" - end - puts %Q(\nSee also "man brew-cask") - end - - def help - "" - end - - def _help_for(klass) - klass.respond_to?(:help) ? klass.help : nil + $stderr.puts + $stderr.puts Help.usage end end end diff --git a/Library/Homebrew/cask/cmd/abstract_command.rb b/Library/Homebrew/cask/cmd/abstract_command.rb index b08a3ea897..c6b988eb4d 100644 --- a/Library/Homebrew/cask/cmd/abstract_command.rb +++ b/Library/Homebrew/cask/cmd/abstract_command.rb @@ -24,7 +24,7 @@ module Cask name.split("::").last.match?(/^Abstract[^a-z]/) end - def self.visible + def self.visible? true end diff --git a/Library/Homebrew/cask/cmd/abstract_internal_command.rb b/Library/Homebrew/cask/cmd/abstract_internal_command.rb index c07fe8b5be..61cfec4fe2 100644 --- a/Library/Homebrew/cask/cmd/abstract_internal_command.rb +++ b/Library/Homebrew/cask/cmd/abstract_internal_command.rb @@ -7,7 +7,7 @@ module Cask super.sub(/^internal_/i, "_") end - def self.visible + def self.visible? false end end diff --git a/Library/Homebrew/cask/cmd/help.rb b/Library/Homebrew/cask/cmd/help.rb new file mode 100644 index 0000000000..51b6ce7876 --- /dev/null +++ b/Library/Homebrew/cask/cmd/help.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Cask + class Cmd + class Help < AbstractCommand + def initialize(*) + super + return if args.empty? + + raise ArgumentError, "#{self.class.command_name} does not take arguments." + end + + def run + puts self.class.purpose + puts + puts self.class.usage + end + + def self.purpose + <<~EOS + Homebrew Cask provides a friendly CLI workflow for the administration + of macOS applications distributed as binaries. + EOS + end + + def self.usage + max_command_len = Cmd.commands.map(&:length).max + + "Commands:\n" + + Cmd.command_classes + .select(&:visible?) + .map { |klass| " #{klass.command_name.ljust(max_command_len)} #{klass.help}\n" } + .join + + %Q(\nSee also "man brew-cask") + end + + def self.help + "print help strings for commands" + end + end + end +end diff --git a/Library/Homebrew/cask/cmd/internal_help.rb b/Library/Homebrew/cask/cmd/internal_help.rb index 2ddac93dbe..ec544a2a06 100644 --- a/Library/Homebrew/cask/cmd/internal_help.rb +++ b/Library/Homebrew/cask/cmd/internal_help.rb @@ -14,17 +14,13 @@ module Cask max_command_len = Cmd.commands.map(&:length).max puts "Unstable Internal-use Commands:\n\n" Cmd.command_classes.each do |klass| - next if klass.visible + next if klass.visible? - puts " #{klass.command_name.ljust(max_command_len)} #{self.class.help_for(klass)}" + puts " #{klass.command_name.ljust(max_command_len)} #{klass.help}" end puts "\n" end - def self.help_for(klass) - klass.respond_to?(:help) ? klass.help : nil - end - def self.help "print help strings for unstable internal-use commands" end diff --git a/Library/Homebrew/test/cask/cmd_spec.rb b/Library/Homebrew/test/cask/cmd_spec.rb index 1674d0e6ae..bd6b9d9795 100644 --- a/Library/Homebrew/test/cask/cmd_spec.rb +++ b/Library/Homebrew/test/cask/cmd_spec.rb @@ -17,7 +17,7 @@ describe Cask::Cmd, :cask do it "ignores the `--language` option, which is handled in `OS::Mac`" do cli = described_class.new("--language=en") - expect(cli).to receive(:detect_command_and_arguments).with(no_args) + expect(cli).to receive(:detect_internal_command).with(no_args) cli.run end @@ -36,28 +36,18 @@ describe Cask::Cmd, :cask do end context "::run" do - let(:noop_command) { double("Cmd::Noop") } - - before do - allow(described_class).to receive(:lookup_command).with("noop").and_return(noop_command) - allow(noop_command).to receive(:run) - end - - it "passes `--version` along to the subcommand" do - version_command = double("Cmd::Version") - allow(described_class).to receive(:lookup_command).with("--version").and_return(version_command) - expect(described_class).to receive(:run_command).with(version_command) - described_class.run("--version") - end + let(:noop_command) { double("Cmd::Noop", run: nil) } it "prints help output when subcommand receives `--help` flag" do - command = described_class.new("noop", "--help") - expect(described_class).to receive(:run_command).with("help", "noop") - command.run + command = described_class.new("info", "--help") + + expect { command.run }.to output(/displays information about the given Cask/).to_stdout expect(command.help?).to eq(true) end it "respects the env variable when choosing what appdir to create" do + allow(described_class).to receive(:lookup_command).with("noop").and_return(noop_command) + ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/custom/appdir" described_class.run("noop") @@ -65,6 +55,16 @@ describe Cask::Cmd, :cask do expect(Cask::Config.global.appdir).to eq(Pathname.new("/custom/appdir")) end + it "overrides the env variable when passing --appdir directly" do + allow(described_class).to receive(:lookup_command).with("noop").and_return(noop_command) + + ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/custom/appdir" + + described_class.run("noop", "--appdir=/even/more/custom/appdir") + + expect(Cask::Config.global.appdir).to eq(Pathname.new("/even/more/custom/appdir")) + end + it "exits with a status of 1 when something goes wrong" do allow(described_class).to receive(:lookup_command).and_raise(Cask::CaskError) command = described_class.new("noop") @@ -73,8 +73,8 @@ describe Cask::Cmd, :cask do end end - it "provides a help message for all visible commands" do - described_class.command_classes.select(&:visible).each do |command_class| + it "provides a help message for all commands" do + described_class.command_classes.each do |command_class| expect(command_class.help).to match(/\w+/), command_class.name end end