From d18b122272f8b67b62f0398f730a9a0faf3655b7 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Sun, 22 Sep 2019 20:13:11 +0530 Subject: [PATCH 1/3] cli_args: Fix options_only and flags_only --- Library/Homebrew/cli/args.rb | 36 ++++++++++++++++++++++++ Library/Homebrew/cli/parser.rb | 1 + Library/Homebrew/test/cli/parser_spec.rb | 21 ++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/Library/Homebrew/cli/args.rb b/Library/Homebrew/cli/args.rb index bcb3563cd4..a85d1e5a07 100644 --- a/Library/Homebrew/cli/args.rb +++ b/Library/Homebrew/cli/args.rb @@ -5,12 +5,48 @@ require "ostruct" module Homebrew module CLI class Args < OpenStruct + attr_accessor :processed_options # undefine tap to allow --tap argument undef tap def initialize(argv:) super @argv = argv + @processed_options = [] + end + + def option_to_name(option) + option.sub(/\A--?/, "") + .tr("-", "_") + end + + def cli_args + return @cli_args unless @cli_args.nil? + + @cli_args = [] + processed_options.each do |short, long| + option = long || short + switch = "#{option_to_name(option)}?".to_sym + flag = option_to_name(option).to_sym + if @table[switch].instance_of? TrueClass + @cli_args << option + elsif @table[flag].instance_of? TrueClass + @cli_args << option + elsif @table[flag].instance_of? String + @cli_args << option + "=" + @table[flag] + elsif @table[flag].instance_of? Array + @cli_args << option + "=" + @table[flag].join(",") + end + end + @cli_args + end + + def options_only + @options_only ||= cli_args.select { |arg| arg.start_with?("-") } + end + + def flags_only + @flags_only ||= cli_args.select { |arg| arg.start_with?("--") } end end end diff --git a/Library/Homebrew/cli/parser.rb b/Library/Homebrew/cli/parser.rb index 5d69ae0cf4..1e76709170 100644 --- a/Library/Homebrew/cli/parser.rb +++ b/Library/Homebrew/cli/parser.rb @@ -139,6 +139,7 @@ module Homebrew check_constraint_violations @args[:remaining] = remaining_args @args_parsed = true + @args.processed_options = @processed_options Homebrew.args = @args cmdline_args.freeze @parser diff --git a/Library/Homebrew/test/cli/parser_spec.rb b/Library/Homebrew/test/cli/parser_spec.rb index a0cfa8c5cf..d0ef7ad2fc 100644 --- a/Library/Homebrew/test/cli/parser_spec.rb +++ b/Library/Homebrew/test/cli/parser_spec.rb @@ -210,4 +210,25 @@ describe Homebrew::CLI::Parser do expect { parser.parse(["--switch-b"]) }.to raise_error(RuntimeError, /Arguments were already parsed!/) end end + + describe "test argv extensions" do + subject(:parser) { + described_class.new do + switch "--foo" + flag "--bar" + switch "-s" + switch :verbose + end + } + + it "#options_only" do + parser.parse(["--foo", "--bar=value", "-v", "-s", "a", "b", "cdefg"]) + expect(Homebrew.args.options_only).to eq %w[--foo --bar=value -s --verbose] + end + + it "#flags_only" do + parser.parse(["--foo", "--bar=value", "-v", "-s", "a", "b", "cdefg"]) + expect(Homebrew.args.flags_only).to eq %w[--foo --bar=value --verbose] + end + end end From 393c8dfbf1577d40478e289a66b107818e80aef3 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Mon, 23 Sep 2019 12:36:45 +0530 Subject: [PATCH 2/3] ARGV: Replace options_only and flags_only with Homebrew.args counterparts --- Library/Homebrew/cmd/cat.rb | 3 ++- Library/Homebrew/cmd/list.rb | 3 ++- Library/Homebrew/cmd/log.rb | 2 +- Library/Homebrew/cmd/upgrade.rb | 2 +- Library/Homebrew/dev-cmd/test.rb | 2 +- Library/Homebrew/reinstall.rb | 2 +- 6 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cmd/cat.rb b/Library/Homebrew/cmd/cat.rb index a8c360dcc0..8c949033a3 100644 --- a/Library/Homebrew/cmd/cat.rb +++ b/Library/Homebrew/cmd/cat.rb @@ -25,6 +25,7 @@ module Homebrew raise "`brew cat` doesn't support multiple arguments" if args.remaining.size > 1 cd HOMEBREW_REPOSITORY - safe_system "cat", formulae.first.path, *ARGV.options_only + cat_args = Homebrew.args.options_only - CLI::Parser.global_options.values.map(&:first).flatten + safe_system "cat", formulae.first.path, *cat_args end end diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index cde69c3ad5..d02b47a893 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -70,7 +70,8 @@ module Homebrew puts Formatter.columns(full_names) else ENV["CLICOLOR"] = nil - safe_system "ls", *ARGV.options_only << HOMEBREW_CELLAR + ls_args = Homebrew.args.options_only - CLI::Parser.global_options.values.map(&:first).flatten + safe_system "ls", *ls_args << HOMEBREW_CELLAR end elsif args.verbose? || !$stdout.tty? system_command! "find", args: ARGV.kegs.map(&:to_s) + %w[-not -type d -print], print_stdout: true diff --git a/Library/Homebrew/cmd/log.rb b/Library/Homebrew/cmd/log.rb index 58c70ec8b0..84dab11a10 100644 --- a/Library/Homebrew/cmd/log.rb +++ b/Library/Homebrew/cmd/log.rb @@ -57,7 +57,7 @@ module Homebrew git -C "#{git_cd}" fetch --unshallow EOS end - args = ARGV.options_only + args = Homebrew.args.options_only args += ["--follow", "--", path] unless path.nil? system "git", "log", *args end diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index c19cce8f0b..e1488c2fd7 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -162,7 +162,7 @@ module Homebrew tab = Tab.for_keg(keg) end - build_options = BuildOptions.new(Options.create(ARGV.flags_only), f.options) + build_options = BuildOptions.new(Options.create(Homebrew.args.flags_only), f.options) options = build_options.used_options options |= f.build.used_options options &= f.options diff --git a/Library/Homebrew/dev-cmd/test.rb b/Library/Homebrew/dev-cmd/test.rb index 1d5ad41103..17eca7dab6 100644 --- a/Library/Homebrew/dev-cmd/test.rb +++ b/Library/Homebrew/dev-cmd/test.rb @@ -82,7 +82,7 @@ module Homebrew -- #{HOMEBREW_LIBRARY_PATH}/test.rb #{f.path} - ].concat(ARGV.options_only) + ].concat(Homebrew.args.options_only) if f.head? args << "--HEAD" diff --git a/Library/Homebrew/reinstall.rb b/Library/Homebrew/reinstall.rb index 51862302a1..067d8b38e8 100644 --- a/Library/Homebrew/reinstall.rb +++ b/Library/Homebrew/reinstall.rb @@ -16,7 +16,7 @@ module Homebrew backup keg end - build_options = BuildOptions.new(Options.create(ARGV.flags_only), f.options) + build_options = BuildOptions.new(Options.create(Homebrew.args.flags_only), f.options) options = build_options.used_options options |= f.build.used_options options &= f.options From e88f6b9da9fab7dde364a8049b023b736b6ef002 Mon Sep 17 00:00:00 2001 From: Gautham Goli Date: Wed, 25 Sep 2019 14:21:06 +0530 Subject: [PATCH 3/3] args: Add passthrough method and tests --- Library/Homebrew/cli/args.rb | 10 ++++++---- Library/Homebrew/cmd/cat.rb | 3 +-- Library/Homebrew/cmd/list.rb | 3 +-- Library/Homebrew/test/cli/parser_spec.rb | 5 +++++ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/cli/args.rb b/Library/Homebrew/cli/args.rb index a85d1e5a07..cc2f277e02 100644 --- a/Library/Homebrew/cli/args.rb +++ b/Library/Homebrew/cli/args.rb @@ -21,16 +21,14 @@ module Homebrew end def cli_args - return @cli_args unless @cli_args.nil? + return @cli_args if @cli_args @cli_args = [] processed_options.each do |short, long| option = long || short switch = "#{option_to_name(option)}?".to_sym flag = option_to_name(option).to_sym - if @table[switch].instance_of? TrueClass - @cli_args << option - elsif @table[flag].instance_of? TrueClass + if @table[switch] == true || @table[flag] == true @cli_args << option elsif @table[flag].instance_of? String @cli_args << option + "=" + @table[flag] @@ -48,6 +46,10 @@ module Homebrew def flags_only @flags_only ||= cli_args.select { |arg| arg.start_with?("--") } end + + def passthrough + options_only - CLI::Parser.global_options.values.map(&:first).flatten + end end end end diff --git a/Library/Homebrew/cmd/cat.rb b/Library/Homebrew/cmd/cat.rb index 8c949033a3..8075034046 100644 --- a/Library/Homebrew/cmd/cat.rb +++ b/Library/Homebrew/cmd/cat.rb @@ -25,7 +25,6 @@ module Homebrew raise "`brew cat` doesn't support multiple arguments" if args.remaining.size > 1 cd HOMEBREW_REPOSITORY - cat_args = Homebrew.args.options_only - CLI::Parser.global_options.values.map(&:first).flatten - safe_system "cat", formulae.first.path, *cat_args + safe_system "cat", formulae.first.path, *Homebrew.args.passthrough end end diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index d02b47a893..a6cd43d331 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -70,8 +70,7 @@ module Homebrew puts Formatter.columns(full_names) else ENV["CLICOLOR"] = nil - ls_args = Homebrew.args.options_only - CLI::Parser.global_options.values.map(&:first).flatten - safe_system "ls", *ls_args << HOMEBREW_CELLAR + safe_system "ls", *Homebrew.args.passthrough << HOMEBREW_CELLAR end elsif args.verbose? || !$stdout.tty? system_command! "find", args: ARGV.kegs.map(&:to_s) + %w[-not -type d -print], print_stdout: true diff --git a/Library/Homebrew/test/cli/parser_spec.rb b/Library/Homebrew/test/cli/parser_spec.rb index d0ef7ad2fc..5d5171f57b 100644 --- a/Library/Homebrew/test/cli/parser_spec.rb +++ b/Library/Homebrew/test/cli/parser_spec.rb @@ -230,5 +230,10 @@ describe Homebrew::CLI::Parser do parser.parse(["--foo", "--bar=value", "-v", "-s", "a", "b", "cdefg"]) expect(Homebrew.args.flags_only).to eq %w[--foo --bar=value --verbose] end + + it "#passthrough" do + parser.parse(["--foo", "--bar=value", "-v", "-s", "a", "b", "cdefg"]) + expect(Homebrew.args.passthrough).to eq %w[--foo --bar=value -s] + end end end