From b2c85ad945b26e43c6e7d36e17bc5e65e30ff5c5 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 1 Jun 2018 14:19:00 +0200 Subject: [PATCH 01/16] Refactor `search`. --- Library/Homebrew/cli_parser.rb | 32 ++++++++++++----- Library/Homebrew/cmd/search.rb | 56 ++++++++++++++++++----------- Library/Homebrew/utils/formatter.rb | 4 +++ 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/Library/Homebrew/cli_parser.rb b/Library/Homebrew/cli_parser.rb index 0ea8af6a66..be8962363f 100644 --- a/Library/Homebrew/cli_parser.rb +++ b/Library/Homebrew/cli_parser.rb @@ -5,8 +5,8 @@ require "set" module Homebrew module CLI class Parser - def self.parse(&block) - new(&block).parse + def self.parse(args = ARGV, &block) + new(&block).parse(args) end def initialize(&block) @@ -60,17 +60,28 @@ module Homebrew @conflicts << options.map { |option| option_to_name(option) } end - def option_to_name(name) - name.sub(/\A--?/, "").tr("-", "_").delete("=") + def option_to_name(option) + option.sub(/\A--?/, "") + .tr("-", "_") + .delete("=") + end + + def name_to_option(name) + if name.length == 1 + "-#{name}" + else + "--#{name}" + end end def option_to_description(*names) names.map { |name| name.to_s.sub(/\A--?/, "").tr("-", " ") }.max end - def parse(cmdline_args = ARGV) - @parser.parse(cmdline_args) + def parse(cmdline_args) + remaining_args = @parser.parse(cmdline_args) check_constraint_violations + Homebrew.args[:remaining] = remaining_args end private @@ -126,7 +137,9 @@ module Homebrew violations = mutually_exclusive_options_group.select do |option| option_passed? option end - raise OptionConflictError, violations if violations.length > 1 + + next if violations.count < 2 + raise OptionConflictError, violations.map(&method(:name_to_option)) end end @@ -163,9 +176,10 @@ module Homebrew class OptionConflictError < RuntimeError def initialize(args) - args_list = args.join("` and `") + args_list = args.map(&Formatter.public_method(:option)) + .join(" and ") super <<~EOS - `#{args_list}` are mutually exclusive + Options #{args_list} are mutually exclusive. EOS end end diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index a9afe5ad7d..5043468884 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -16,31 +16,45 @@ require "formula" require "missing_formula" require "descriptions" +require "cli_parser" module Homebrew module_function - def search - if ARGV.empty? + PACKAGE_MANAGERS = { + macports: ->(query) { "https://www.macports.org/ports.php?by=name&substr=#{query}" }, + fink: ->(query) { "http://pdb.finkproject.org/pdb/browse.php?summary=#{query}" }, + debian: ->(query) { "https://packages.debian.org/search?keywords=#{query}&searchon=names&suite=all§ion=all" }, + opensuse: ->(query) { "https://software.opensuse.org/search?q=#{query}" }, + fedora: ->(query) { "https://apps.fedoraproject.org/packages/s/#{query}" }, + ubuntu: ->(query) { "https://packages.ubuntu.com/search?keywords=#{query}&searchon=names&suite=all§ion=all" }, + }.freeze + + def search(argv = ARGV) + CLI::Parser.parse(argv) do + switch "--desc" + + package_manager_switches = PACKAGE_MANAGERS.keys.map { |name| "--#{name}" } + + package_manager_switches.each do |s| + switch s + end + + conflicts(*package_manager_switches) + end + + PACKAGE_MANAGERS.each do |name, url| + exec_browser url.call(URI.encode_www_form_component(args.remaining.join(" "))) if args[:"#{name}?"] + end + + if args.remaining.empty? puts Formatter.columns(Formula.full_names.sort) - elsif ARGV.include? "--macports" - exec_browser "https://www.macports.org/ports.php?by=name&substr=#{ARGV.next}" - elsif ARGV.include? "--fink" - exec_browser "http://pdb.finkproject.org/pdb/browse.php?summary=#{ARGV.next}" - elsif ARGV.include? "--debian" - exec_browser "https://packages.debian.org/search?keywords=#{ARGV.next}&searchon=names&suite=all§ion=all" - elsif ARGV.include? "--opensuse" - exec_browser "https://software.opensuse.org/search?q=#{ARGV.next}" - elsif ARGV.include? "--fedora" - exec_browser "https://apps.fedoraproject.org/packages/s/#{ARGV.next}" - elsif ARGV.include? "--ubuntu" - exec_browser "https://packages.ubuntu.com/search?keywords=#{ARGV.next}&searchon=names&suite=all§ion=all" - elsif ARGV.include? "--desc" - query = ARGV.next + elsif args.desc? + query = args.remaining.first regex = query_regexp(query) Descriptions.search(regex, :desc).print - elsif ARGV.first =~ HOMEBREW_TAP_FORMULA_REGEX - query = ARGV.first + elsif args.remaining.first =~ HOMEBREW_TAP_FORMULA_REGEX + query = args.remaining.first begin result = Formulary.factory(query).name @@ -52,7 +66,7 @@ module Homebrew puts Formatter.columns(results.sort) unless results.empty? else - query = ARGV.first + query = args.remaining.first regex = query_regexp(query) local_results = search_formulae(regex) puts Formatter.columns(local_results.sort) unless local_results.empty? @@ -78,10 +92,10 @@ module Homebrew end return unless $stdout.tty? - return if ARGV.empty? + return if args.remaining.empty? metacharacters = %w[\\ | ( ) [ ] { } ^ $ * + ?].freeze return unless metacharacters.any? do |char| - ARGV.any? do |arg| + args.remaining.any? do |arg| arg.include?(char) && !arg.start_with?("/") end end diff --git a/Library/Homebrew/utils/formatter.rb b/Library/Homebrew/utils/formatter.rb index a29b43c8de..ec144bf2f8 100644 --- a/Library/Homebrew/utils/formatter.rb +++ b/Library/Homebrew/utils/formatter.rb @@ -15,6 +15,10 @@ module Formatter "#{Tty.green}#{string}#{Tty.default}" end + def option(string) + "#{Tty.bold}#{string}#{Tty.reset}" + end + def success(string, label: nil) label(label, string, :green) end From 9e807f8fe41c25088ad30a6aff7c499c7d0d4084 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 1 Jun 2018 18:48:21 +0200 Subject: [PATCH 02/16] Refactor `query_regexp`. --- Library/Homebrew/cmd/search.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 5043468884..63c80e354c 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -106,12 +106,13 @@ module Homebrew end def query_regexp(query) - case query - when %r{^/(.*)/$} then Regexp.new(Regexp.last_match(1)) - else /.*#{Regexp.escape(query)}.*/i + if m = query.match(%r{^/(.*)/$}) + Regexp.new(m[1]) + else + /.*#{Regexp.escape(query)}.*/i end rescue RegexpError - odie "#{query} is not a valid regex" + raise "#{query} is not a valid regex." end def search_taps(query, silent: false) From 8b33fbef5122c27b193dc831362a46a7d64140fc Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 1 Jun 2018 18:48:56 +0200 Subject: [PATCH 03/16] Refactor `search_taps` to return Formulae and Casks separately. --- Library/Homebrew/cmd/search.rb | 50 +++++++++++-------- .../test/cmd/search_remote_tap_spec.rb | 2 +- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 63c80e354c..0f36a3d3df 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -61,18 +61,18 @@ module Homebrew results = Array(result) rescue FormulaUnavailableError _, _, name = query.split("/", 3) - results = search_taps(name) + results = search_taps(name).flatten.sort end - puts Formatter.columns(results.sort) unless results.empty? + puts Formatter.columns(results) unless results.empty? else query = args.remaining.first regex = query_regexp(query) local_results = search_formulae(regex) puts Formatter.columns(local_results.sort) unless local_results.empty? - tap_results = search_taps(query) - puts Formatter.columns(tap_results.sort) unless tap_results.empty? + tap_results = search_taps(query).flatten.sort + puts Formatter.columns(tap_results) unless tap_results.empty? if $stdout.tty? count = local_results.length + tap_results.length @@ -116,30 +116,36 @@ module Homebrew end def search_taps(query, silent: false) - return [] if ENV["HOMEBREW_NO_GITHUB_API"] + return [], [] if ENV["HOMEBREW_NO_GITHUB_API"] - # Use stderr to avoid breaking parsed output unless silent + # Use stderr to avoid breaking parsed output $stderr.puts Formatter.headline("Searching taps on GitHub...", color: :blue) end - matches = begin - GitHub.search_code( - user: "Homebrew", - path: ["Formula", "HomebrewFormula", "Casks", "."], - filename: query, - extension: "rb", - ) - rescue GitHub::Error => error - opoo "Error searching on GitHub: #{error}\n" - [] - end - matches.map do |match| - filename = File.basename(match["path"], ".rb") + matches = GitHub.search_code( + user: "Homebrew", + path: ["Formula", "Casks", "."], + filename: query, + extension: "rb", + ) + + matches.inject([[], []]) do |(formulae, casks), match| + name = File.basename(match["path"], ".rb") tap = Tap.fetch(match["repository"]["full_name"]) - next if tap.installed? && !tap.name.start_with?("homebrew/cask") - "#{tap.name}/#{filename}" - end.compact + full_name = "#{tap.name}/#{name}" + + if tap.installed? + [formulae, casks] + elsif match["path"].start_with?("Casks/") + [formulae, [*casks, full_name].sort] + else + [[*formulae, full_name].sort, casks] + end + end + rescue GitHub::Error => error + opoo "Error searching on GitHub: #{error}\n" + [[], []] end def search_formulae(regex) diff --git a/Library/Homebrew/test/cmd/search_remote_tap_spec.rb b/Library/Homebrew/test/cmd/search_remote_tap_spec.rb index 49678f3ace..c47b448bb3 100644 --- a/Library/Homebrew/test/cmd/search_remote_tap_spec.rb +++ b/Library/Homebrew/test/cmd/search_remote_tap_spec.rb @@ -19,6 +19,6 @@ describe Homebrew do allow(GitHub).to receive(:open_api).and_yield(json_response) expect(described_class.search_taps("some-formula")) - .to match(["homebrew/foo/some-formula"]) + .to match([["homebrew/foo/some-formula"], []]) end end From 3a361c139e9dad4874c17cb254a8657140354195 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 02:12:40 +0200 Subject: [PATCH 04/16] Extend `search` tests. --- .../test/cmd/search_remote_tap_spec.rb | 51 +++++++++++++------ Library/Homebrew/test/cmd/search_spec.rb | 16 ++++++ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/test/cmd/search_remote_tap_spec.rb b/Library/Homebrew/test/cmd/search_remote_tap_spec.rb index c47b448bb3..41acc5a690 100644 --- a/Library/Homebrew/test/cmd/search_remote_tap_spec.rb +++ b/Library/Homebrew/test/cmd/search_remote_tap_spec.rb @@ -1,24 +1,45 @@ require "cmd/search" describe Homebrew do - specify "#search_taps" do - # Otherwise the tested method returns [], regardless of our stub - ENV.delete("HOMEBREW_NO_GITHUB_API") + describe "#search_taps" do + before do + ENV.delete("HOMEBREW_NO_GITHUB_API") + end - json_response = { - "items" => [ - { - "path" => "Formula/some-formula.rb", - "repository" => { - "full_name" => "Homebrew/homebrew-foo", + it "does not raise if `HOMEBREW_NO_GITHUB_API` is set" do + ENV["HOMEBREW_NO_GITHUB_API"] = "1" + expect(described_class.search_taps("some-formula")).to match([[], []]) + end + + it "does not raise if the network fails" do + allow(GitHub).to receive(:open_api).and_raise(GitHub::Error) + + expect(described_class.search_taps("some-formula")) + .to match([[], []]) + end + + it "returns Formulae and Casks separately" do + json_response = { + "items" => [ + { + "path" => "Formula/some-formula.rb", + "repository" => { + "full_name" => "Homebrew/homebrew-foo", + }, }, - }, - ], - } + { + "path" => "Casks/some-cask.rb", + "repository" => { + "full_name" => "Homebrew/homebrew-bar", + }, + }, + ], + } - allow(GitHub).to receive(:open_api).and_yield(json_response) + allow(GitHub).to receive(:open_api).and_yield(json_response) - expect(described_class.search_taps("some-formula")) - .to match([["homebrew/foo/some-formula"], []]) + expect(described_class.search_taps("some-formula")) + .to match([["homebrew/foo/some-formula"], ["homebrew/bar/some-cask"]]) + end end end diff --git a/Library/Homebrew/test/cmd/search_spec.rb b/Library/Homebrew/test/cmd/search_spec.rb index 1e1ef8e180..9e77fc6b61 100644 --- a/Library/Homebrew/test/cmd/search_spec.rb +++ b/Library/Homebrew/test/cmd/search_spec.rb @@ -1,3 +1,5 @@ +require "cmd/search" + describe "brew search", :integration_test do before do setup_test_formula "testball" @@ -63,4 +65,18 @@ describe "brew search", :integration_test do .and be_a_success end end + + describe "::query_regexp" do + it "correctly parses a regex query" do + expect(Homebrew.query_regexp("/^query$/")).to eq(/^query$/) + end + + it "correctly converts a query string to a regex" do + expect(Homebrew.query_regexp("query")).to eq(/.*query.*/i) + end + + it "raises an error if the query is an invalid regex" do + expect { Homebrew.query_regexp("/+/") }.to raise_error(/not a valid regex/) + end + end end From e786bb08c940358b3f979042bd0c749d51d55dba Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 02:25:20 +0200 Subject: [PATCH 05/16] Use `search_taps` in `brew cask search`. --- Library/Homebrew/cask/lib/hbc/cli/search.rb | 24 +++------------------ 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/search.rb b/Library/Homebrew/cask/lib/hbc/cli/search.rb index 3decd6bd8d..9256317413 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/search.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/search.rb @@ -1,3 +1,5 @@ +require "cmd/search" + module Hbc class CLI class Search < AbstractCommand @@ -18,26 +20,6 @@ module Hbc end end - def self.search_remote(query) - matches = begin - GitHub.search_code( - user: "Homebrew", - path: "Casks", - filename: query, - extension: "rb", - ) - rescue GitHub::Error => error - opoo "Error searching on GitHub: #{error}\n" - [] - end - - matches.map do |match| - tap = Tap.fetch(match["repository"]["full_name"]) - next if tap.installed? - "#{tap.name}/#{File.basename(match["path"], ".rb")}" - end.compact - end - def self.search(*arguments) exact_match = nil partial_matches = [] @@ -55,7 +37,7 @@ module Hbc partial_matches.delete(exact_match) end - remote_matches = search_remote(search_term) + _, remote_matches = Homebrew.search_taps(search_term, silent: true) [exact_match, partial_matches, remote_matches, search_term] end From 14b3b82fcaa5c3e44c5e89de6b5ca802218f4a0e Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 02:47:23 +0200 Subject: [PATCH 06/16] Remove exact match from `brew cask search`. --- .../cask/lib/hbc/cli/abstract_command.rb | 20 +++++++------------ Library/Homebrew/cask/lib/hbc/cli/search.rb | 17 +++++----------- Library/Homebrew/test/cask/cli/search_spec.rb | 5 ++--- 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb b/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb index 001a9623b2..2cfe3c467d 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/abstract_command.rb @@ -9,7 +9,7 @@ module Hbc option "--debug", :debug, false option "--verbose", :verbose, false option "--outdated", :outdated_only, false - option "--require-sha", :require_sha, false + option "--require-sha", :require_sha, false def self.command_name @command_name ||= name.sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1_\2').downcase @@ -49,24 +49,18 @@ module Hbc casks = args.empty? ? alternative.call : args @casks = casks.map { |cask| CaskLoader.load(cask) } rescue CaskUnavailableError => e - reason = [e.reason, suggestion_message(e.token)].join(" ") + 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) + matches, = Search.search(cask_token) - if exact_match.nil? && partial_matches.count == 1 - exact_match = partial_matches.first - end - - if exact_match - "Did you mean “#{exact_match}”?" - elsif !partial_matches.empty? + if matches.one? + "Did you mean “#{matches.first}”?" + elsif !matches.empty? "Did you mean one of these?\n" - .concat(Formatter.columns(partial_matches.take(20))) - else - "" + .concat(Formatter.columns(matches.take(20))) end end end diff --git a/Library/Homebrew/cask/lib/hbc/cli/search.rb b/Library/Homebrew/cask/lib/hbc/cli/search.rb index 9256317413..4fbecccbf6 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/search.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/search.rb @@ -21,7 +21,6 @@ module Hbc end def self.search(*arguments) - exact_match = nil partial_matches = [] search_term = arguments.join(" ") search_regexp = extract_regexp arguments.first @@ -32,36 +31,30 @@ module Hbc else simplified_tokens = all_tokens.map { |t| t.sub(%r{^.*\/}, "").gsub(/[^a-z0-9]+/i, "") } simplified_search_term = search_term.sub(/\.rb$/i, "").gsub(/[^a-z0-9]+/i, "") - exact_match = simplified_tokens.grep(/^#{simplified_search_term}$/i) { |t| all_tokens[simplified_tokens.index(t)] }.first partial_matches = simplified_tokens.grep(/#{simplified_search_term}/i) { |t| all_tokens[simplified_tokens.index(t)] } - partial_matches.delete(exact_match) end _, remote_matches = Homebrew.search_taps(search_term, silent: true) - [exact_match, partial_matches, remote_matches, search_term] + [partial_matches, remote_matches, search_term] end - def self.render_results(exact_match, partial_matches, remote_matches, search_term) + def self.render_results(partial_matches, remote_matches, search_term) unless $stdout.tty? - puts [*exact_match, *partial_matches, *remote_matches] + puts [*partial_matches, *remote_matches] return end - if !exact_match && partial_matches.empty? && remote_matches.empty? + if partial_matches.empty? && remote_matches.empty? puts "No Cask found for \"#{search_term}\"." return end - if exact_match - ohai "Exact Match" - puts highlight_installed exact_match - end unless partial_matches.empty? if extract_regexp search_term ohai "Regexp Matches" else - ohai "Partial Matches" + ohai "Matches" end puts Formatter.columns(partial_matches.map(&method(:highlight_installed))) end diff --git a/Library/Homebrew/test/cask/cli/search_spec.rb b/Library/Homebrew/test/cask/cli/search_spec.rb index 5d8ad30e4f..5500500b81 100644 --- a/Library/Homebrew/test/cask/cli/search_spec.rb +++ b/Library/Homebrew/test/cask/cli/search_spec.rb @@ -13,7 +13,7 @@ describe Hbc::CLI::Search, :cask do expect { Hbc::CLI::Search.run("local") }.to output(<<~EOS).to_stdout.as_tty - ==> Partial Matches + ==> Matches local-caffeine local-transmission EOS @@ -94,9 +94,8 @@ describe Hbc::CLI::Search, :cask do expect { Hbc::CLI::Search.run("test-opera") }.to output(<<~EOS).to_stdout.as_tty - ==> Exact Match + ==> Matches test-opera - ==> Partial Matches test-opera-mail EOS end From 49619b09ea0dd594cce493675bf3b19d227358d8 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 20:50:18 +0200 Subject: [PATCH 07/16] Refactor package manager loop. --- Library/Homebrew/cmd/search.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 0f36a3d3df..5ed4c201c9 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -43,8 +43,10 @@ module Homebrew conflicts(*package_manager_switches) end - PACKAGE_MANAGERS.each do |name, url| - exec_browser url.call(URI.encode_www_form_component(args.remaining.join(" "))) if args[:"#{name}?"] + if package_manager = PACKAGE_MANAGERS.detect { |name,| args[:"#{name}?"] } + _, url = package_manager + exec_browser url.call(URI.encode_www_form_component(args.remaining.join(" "))) + return end if args.remaining.empty? From dd8370a9faca3dbf89688380fda73b51c50ae936 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 20:49:14 +0200 Subject: [PATCH 08/16] Move shared search logic into `Homebrew::Search` module. --- Library/Homebrew/cask/lib/hbc/cli/search.rb | 6 +- Library/Homebrew/cmd/search.rb | 73 +------------------ Library/Homebrew/search.rb | 73 +++++++++++++++++++ Library/Homebrew/test/cmd/search_spec.rb | 14 ---- ...arch_remote_tap_spec.rb => search_spec.rb} | 30 ++++++-- 5 files changed, 105 insertions(+), 91 deletions(-) create mode 100644 Library/Homebrew/search.rb rename Library/Homebrew/test/{cmd/search_remote_tap_spec.rb => search_spec.rb} (58%) diff --git a/Library/Homebrew/cask/lib/hbc/cli/search.rb b/Library/Homebrew/cask/lib/hbc/cli/search.rb index 4fbecccbf6..addc35c98c 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/search.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/search.rb @@ -1,8 +1,10 @@ -require "cmd/search" +require "search" module Hbc class CLI class Search < AbstractCommand + extend Homebrew::Search + def run if args.empty? puts Formatter.columns(CLI.nice_listing(Cask.map(&:qualified_token))) @@ -34,7 +36,7 @@ module Hbc partial_matches = simplified_tokens.grep(/#{simplified_search_term}/i) { |t| all_tokens[simplified_tokens.index(t)] } end - _, remote_matches = Homebrew.search_taps(search_term, silent: true) + _, remote_matches = search_taps(search_term, silent: true) [partial_matches, remote_matches, search_term] end diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 5ed4c201c9..4290b077e8 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -17,10 +17,13 @@ require "formula" require "missing_formula" require "descriptions" require "cli_parser" +require "search" module Homebrew module_function + extend Search + PACKAGE_MANAGERS = { macports: ->(query) { "https://www.macports.org/ports.php?by=name&substr=#{query}" }, fink: ->(query) { "http://pdb.finkproject.org/pdb/browse.php?summary=#{query}" }, @@ -106,74 +109,4 @@ module Homebrew Surround your query with /slashes/ to search locally by regex. EOS end - - def query_regexp(query) - if m = query.match(%r{^/(.*)/$}) - Regexp.new(m[1]) - else - /.*#{Regexp.escape(query)}.*/i - end - rescue RegexpError - raise "#{query} is not a valid regex." - end - - def search_taps(query, silent: false) - return [], [] if ENV["HOMEBREW_NO_GITHUB_API"] - - unless silent - # Use stderr to avoid breaking parsed output - $stderr.puts Formatter.headline("Searching taps on GitHub...", color: :blue) - end - - matches = GitHub.search_code( - user: "Homebrew", - path: ["Formula", "Casks", "."], - filename: query, - extension: "rb", - ) - - matches.inject([[], []]) do |(formulae, casks), match| - name = File.basename(match["path"], ".rb") - tap = Tap.fetch(match["repository"]["full_name"]) - full_name = "#{tap.name}/#{name}" - - if tap.installed? - [formulae, casks] - elsif match["path"].start_with?("Casks/") - [formulae, [*casks, full_name].sort] - else - [[*formulae, full_name].sort, casks] - end - end - rescue GitHub::Error => error - opoo "Error searching on GitHub: #{error}\n" - [[], []] - end - - def search_formulae(regex) - # Use stderr to avoid breaking parsed output - $stderr.puts Formatter.headline("Searching local taps...", color: :blue) - - aliases = Formula.alias_full_names - results = (Formula.full_names + aliases).grep(regex).sort - - results.map do |name| - begin - formula = Formulary.factory(name) - canonical_name = formula.name - canonical_full_name = formula.full_name - rescue - canonical_name = canonical_full_name = name - end - - # Ignore aliases from results when the full name was also found - next if aliases.include?(name) && results.include?(canonical_full_name) - - if (HOMEBREW_CELLAR/canonical_name).directory? - pretty_installed(name) - else - name - end - end.compact - end end diff --git a/Library/Homebrew/search.rb b/Library/Homebrew/search.rb new file mode 100644 index 0000000000..e00826506f --- /dev/null +++ b/Library/Homebrew/search.rb @@ -0,0 +1,73 @@ +module Homebrew + module Search + def query_regexp(query) + if m = query.match(%r{^/(.*)/$}) + Regexp.new(m[1]) + else + /.*#{Regexp.escape(query)}.*/i + end + rescue RegexpError + raise "#{query} is not a valid regex." + end + + def search_taps(query, silent: false) + return [], [] if ENV["HOMEBREW_NO_GITHUB_API"] + + unless silent + # Use stderr to avoid breaking parsed output + $stderr.puts Formatter.headline("Searching taps on GitHub...", color: :blue) + end + + matches = GitHub.search_code( + user: "Homebrew", + path: ["Formula", "Casks", "."], + filename: query, + extension: "rb", + ) + + matches.inject([[], []]) do |(formulae, casks), match| + name = File.basename(match["path"], ".rb") + tap = Tap.fetch(match["repository"]["full_name"]) + full_name = "#{tap.name}/#{name}" + + if tap.installed? + [formulae, casks] + elsif match["path"].start_with?("Casks/") + [formulae, [*casks, full_name].sort] + else + [[*formulae, full_name].sort, casks] + end + end + rescue GitHub::Error => error + opoo "Error searching on GitHub: #{error}\n" + [[], []] + end + + def search_formulae(regex) + # Use stderr to avoid breaking parsed output + $stderr.puts Formatter.headline("Searching local taps...", color: :blue) + + aliases = Formula.alias_full_names + results = (Formula.full_names + aliases).grep(regex).sort + + results.map do |name| + begin + formula = Formulary.factory(name) + canonical_name = formula.name + canonical_full_name = formula.full_name + rescue + canonical_name = canonical_full_name = name + end + + # Ignore aliases from results when the full name was also found + next if aliases.include?(name) && results.include?(canonical_full_name) + + if (HOMEBREW_CELLAR/canonical_name).directory? + pretty_installed(name) + else + name + end + end.compact + end + end +end diff --git a/Library/Homebrew/test/cmd/search_spec.rb b/Library/Homebrew/test/cmd/search_spec.rb index 9e77fc6b61..556c863ca2 100644 --- a/Library/Homebrew/test/cmd/search_spec.rb +++ b/Library/Homebrew/test/cmd/search_spec.rb @@ -65,18 +65,4 @@ describe "brew search", :integration_test do .and be_a_success end end - - describe "::query_regexp" do - it "correctly parses a regex query" do - expect(Homebrew.query_regexp("/^query$/")).to eq(/^query$/) - end - - it "correctly converts a query string to a regex" do - expect(Homebrew.query_regexp("query")).to eq(/.*query.*/i) - end - - it "raises an error if the query is an invalid regex" do - expect { Homebrew.query_regexp("/+/") }.to raise_error(/not a valid regex/) - end - end end diff --git a/Library/Homebrew/test/cmd/search_remote_tap_spec.rb b/Library/Homebrew/test/search_spec.rb similarity index 58% rename from Library/Homebrew/test/cmd/search_remote_tap_spec.rb rename to Library/Homebrew/test/search_spec.rb index 41acc5a690..26236af472 100644 --- a/Library/Homebrew/test/cmd/search_remote_tap_spec.rb +++ b/Library/Homebrew/test/search_spec.rb @@ -1,6 +1,12 @@ -require "cmd/search" +require "search" + +describe Homebrew::Search do + subject(:mod) { Object.new } + + before do + mod.extend(described_class) + end -describe Homebrew do describe "#search_taps" do before do ENV.delete("HOMEBREW_NO_GITHUB_API") @@ -8,13 +14,13 @@ describe Homebrew do it "does not raise if `HOMEBREW_NO_GITHUB_API` is set" do ENV["HOMEBREW_NO_GITHUB_API"] = "1" - expect(described_class.search_taps("some-formula")).to match([[], []]) + expect(mod.search_taps("some-formula")).to match([[], []]) end it "does not raise if the network fails" do allow(GitHub).to receive(:open_api).and_raise(GitHub::Error) - expect(described_class.search_taps("some-formula")) + expect(mod.search_taps("some-formula")) .to match([[], []]) end @@ -38,8 +44,22 @@ describe Homebrew do allow(GitHub).to receive(:open_api).and_yield(json_response) - expect(described_class.search_taps("some-formula")) + expect(mod.search_taps("some-formula")) .to match([["homebrew/foo/some-formula"], ["homebrew/bar/some-cask"]]) end end + + describe "#query_regexp" do + it "correctly parses a regex query" do + expect(mod.query_regexp("/^query$/")).to eq(/^query$/) + end + + it "correctly converts a query string to a regex" do + expect(mod.query_regexp("query")).to eq(/.*query.*/i) + end + + it "raises an error if the query is an invalid regex" do + expect { mod.query_regexp("/+/") }.to raise_error(/not a valid regex/) + end + end end From cff42a8a8cc7be31f159c9ed9f3e940940a53ac0 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 21:00:48 +0200 Subject: [PATCH 09/16] =?UTF-8?q?Show=20remote=20Casks,=20even=20if=20avai?= =?UTF-8?q?lable=20locally=20=E2=80=A6=20for=20now.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Library/Homebrew/search.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/search.rb b/Library/Homebrew/search.rb index e00826506f..1a695ff620 100644 --- a/Library/Homebrew/search.rb +++ b/Library/Homebrew/search.rb @@ -30,7 +30,7 @@ module Homebrew tap = Tap.fetch(match["repository"]["full_name"]) full_name = "#{tap.name}/#{name}" - if tap.installed? + if tap.installed? && !match["path"].start_with?("Casks/") [formulae, casks] elsif match["path"].start_with?("Casks/") [formulae, [*casks, full_name].sort] From 99e3135bfaa73c051c8be01b724f0ed8ad306c75 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Jun 2018 22:43:54 +0200 Subject: [PATCH 10/16] Make `query_regexp` ignore special symbols. --- Library/Homebrew/search.rb | 2 +- Library/Homebrew/test/search_spec.rb | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/search.rb b/Library/Homebrew/search.rb index 1a695ff620..fdd90f3ed6 100644 --- a/Library/Homebrew/search.rb +++ b/Library/Homebrew/search.rb @@ -4,7 +4,7 @@ module Homebrew if m = query.match(%r{^/(.*)/$}) Regexp.new(m[1]) else - /.*#{Regexp.escape(query)}.*/i + Regexp.new(query.chars.join('[^a-z\d]*'), Regexp::IGNORECASE) end rescue RegexpError raise "#{query} is not a valid regex." diff --git a/Library/Homebrew/test/search_spec.rb b/Library/Homebrew/test/search_spec.rb index 26236af472..f7be0b805f 100644 --- a/Library/Homebrew/test/search_spec.rb +++ b/Library/Homebrew/test/search_spec.rb @@ -55,11 +55,26 @@ describe Homebrew::Search do end it "correctly converts a query string to a regex" do - expect(mod.query_regexp("query")).to eq(/.*query.*/i) + expect(mod.query_regexp("query")).to eq(/q[^a-z\d]*u[^a-z\d]*e[^a-z\d]*r[^a-z\d]*y/i) end it "raises an error if the query is an invalid regex" do expect { mod.query_regexp("/+/") }.to raise_error(/not a valid regex/) end + + it "correctly matches with special symbols" do + regex = mod.query_regexp("oo-ba") + expect(regex).to match("foo-bar") + end + + it "correctly matches without special symbols" do + regex = mod.query_regexp("ooba") + expect(regex).to match("foo-bar") + end + + it "keeps special symbols" do + regex = mod.query_regexp("foo-bar") + expect(regex).not_to match("foobar") + end end end From 6fcc5d14de042e5328a0d37972af35aa98f5a9eb Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 5 Jun 2018 10:55:00 +0200 Subject: [PATCH 11/16] Simplify strings for search. --- Library/Homebrew/cmd/desc.rb | 8 ++++--- Library/Homebrew/descriptions.rb | 9 +++++--- Library/Homebrew/search.rb | 10 +++++++-- Library/Homebrew/test/search_spec.rb | 31 ++++++++++++++-------------- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/Library/Homebrew/cmd/desc.rb b/Library/Homebrew/cmd/desc.rb index ccb7e99255..b704d08b85 100644 --- a/Library/Homebrew/cmd/desc.rb +++ b/Library/Homebrew/cmd/desc.rb @@ -9,11 +9,13 @@ #: first search, making that search slower than subsequent ones. require "descriptions" -require "cmd/search" +require "search" module Homebrew module_function + extend Search + def desc search_type = [] search_type << :either if ARGV.flag? "--search" @@ -28,8 +30,8 @@ module Homebrew results.print elsif search_type.size > 1 odie "Pick one, and only one, of -s/--search, -n/--name, or -d/--description." - elsif arg = ARGV.named.first - regex = Homebrew.query_regexp(arg) + elsif arg = ARGV.named.join(" ") + regex = query_regexp(arg) results = Descriptions.search(regex, search_type.first) results.print else diff --git a/Library/Homebrew/descriptions.rb b/Library/Homebrew/descriptions.rb index 4db47e77f1..9d7c16b636 100644 --- a/Library/Homebrew/descriptions.rb +++ b/Library/Homebrew/descriptions.rb @@ -1,7 +1,10 @@ require "formula" require "formula_versions" +require "search" class Descriptions + extend Homebrew::Search + CACHE_FILE = HOMEBREW_CACHE + "desc_cache.json" def self.cache @@ -99,11 +102,11 @@ class Descriptions results = case field when :name - @cache.select { |name, _| name =~ regex } + @cache.select { |name, _| simplify_string(name).match?(regex) } when :desc - @cache.select { |_, desc| desc =~ regex } + @cache.select { |_, desc| simplify_string(desc).match?(regex) } when :either - @cache.select { |name, desc| (name =~ regex) || (desc =~ regex) } + @cache.select { |name, desc| simplify_string(name).match?(regex) || simplify_string(desc).match?(regex) } end new(results) diff --git a/Library/Homebrew/search.rb b/Library/Homebrew/search.rb index fdd90f3ed6..c94a5a61f8 100644 --- a/Library/Homebrew/search.rb +++ b/Library/Homebrew/search.rb @@ -1,10 +1,14 @@ module Homebrew module Search + def simplify_string(string) + string.downcase.gsub(/[^a-z\d]/i, "") + end + def query_regexp(query) if m = query.match(%r{^/(.*)/$}) Regexp.new(m[1]) else - Regexp.new(query.chars.join('[^a-z\d]*'), Regexp::IGNORECASE) + Regexp.new(simplify_string(query), Regexp::IGNORECASE) end rescue RegexpError raise "#{query} is not a valid regex." @@ -48,7 +52,9 @@ module Homebrew $stderr.puts Formatter.headline("Searching local taps...", color: :blue) aliases = Formula.alias_full_names - results = (Formula.full_names + aliases).grep(regex).sort + results = (Formula.full_names + aliases) + .select { |name| simplify_string(name).match?(regex) } + .sort results.map do |name| begin diff --git a/Library/Homebrew/test/search_spec.rb b/Library/Homebrew/test/search_spec.rb index f7be0b805f..222ace3080 100644 --- a/Library/Homebrew/test/search_spec.rb +++ b/Library/Homebrew/test/search_spec.rb @@ -49,32 +49,31 @@ describe Homebrew::Search do end end + describe "#simplify_string" do + it "simplifies a query with dashes" do + expect(mod.query_regexp("que-ry")).to eq(/query/i) + end + + it "simplifies a query with @ symbols" do + expect(mod.query_regexp("query@1")).to eq(/query1/i) + end + end + describe "#query_regexp" do it "correctly parses a regex query" do expect(mod.query_regexp("/^query$/")).to eq(/^query$/) end it "correctly converts a query string to a regex" do - expect(mod.query_regexp("query")).to eq(/q[^a-z\d]*u[^a-z\d]*e[^a-z\d]*r[^a-z\d]*y/i) + expect(mod.query_regexp("query")).to eq(/query/i) + end + + it "simplifies a query with special symbols" do + expect(mod.query_regexp("que-ry")).to eq(/query/i) end it "raises an error if the query is an invalid regex" do expect { mod.query_regexp("/+/") }.to raise_error(/not a valid regex/) end - - it "correctly matches with special symbols" do - regex = mod.query_regexp("oo-ba") - expect(regex).to match("foo-bar") - end - - it "correctly matches without special symbols" do - regex = mod.query_regexp("ooba") - expect(regex).to match("foo-bar") - end - - it "keeps special symbols" do - regex = mod.query_regexp("foo-bar") - expect(regex).not_to match("foobar") - end end end From 46e0de1762c8b2b8d5dfaa4953e9b031489a9a6b Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 5 Jun 2018 14:21:05 +0200 Subject: [PATCH 12/16] Add `Searchable` helper module. --- Library/Homebrew/searchable.rb | 31 ++++++++++++++++++++++++ Library/Homebrew/test/searchable_spec.rb | 30 +++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 Library/Homebrew/searchable.rb create mode 100644 Library/Homebrew/test/searchable_spec.rb diff --git a/Library/Homebrew/searchable.rb b/Library/Homebrew/searchable.rb new file mode 100644 index 0000000000..9fca0bfdaa --- /dev/null +++ b/Library/Homebrew/searchable.rb @@ -0,0 +1,31 @@ +module Searchable + def search(string_or_regex, &block) + case string_or_regex + when Regexp + search_regex(string_or_regex, &block) + else + search_string(string_or_regex.to_str, &block) + end + end + + private + + def simplify_string(string) + string.downcase.gsub(/[^a-z\d]/i, "") + end + + def search_regex(regex) + select do |*args| + args = yield(*args) if block_given? + [*args].any? { |arg| arg.match?(regex) } + end + end + + def search_string(string) + simplified_string = simplify_string(string) + select do |*args| + args = yield(*args) if block_given? + [*args].any? { |arg| simplify_string(arg).include?(simplified_string) } + end + end +end diff --git a/Library/Homebrew/test/searchable_spec.rb b/Library/Homebrew/test/searchable_spec.rb new file mode 100644 index 0000000000..aaf2247795 --- /dev/null +++ b/Library/Homebrew/test/searchable_spec.rb @@ -0,0 +1,30 @@ +require "searchable" + +describe Searchable do + subject { ary.extend(described_class) } + + let(:ary) { ["with-dashes"] } + + describe "#search" do + context "when given a block" do + let(:ary) { [["with-dashes", "withdashes"]] } + + it "searches by the selected argument" do + expect(subject.search(/withdashes/) { |_, short_name| short_name }).not_to be_empty + expect(subject.search(/withdashes/) { |long_name, _| long_name }).to be_empty + end + end + + context "when given a regex" do + it "does not simplify strings" do + expect(subject.search(/with\-dashes/)).to eq ["with-dashes"] + end + end + + context "when given a string" do + it "simplifies both the query and searched strings" do + expect(subject.search("with dashes")).to eq ["with-dashes"] + end + end + end +end From 717032d86dd458539004b67e1ac883937df0121f Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 5 Jun 2018 15:39:09 +0200 Subject: [PATCH 13/16] Use `Searchable` module. --- Library/Homebrew/cmd/desc.rb | 7 ++++--- Library/Homebrew/cmd/install.rb | 8 ++++---- Library/Homebrew/cmd/search.rb | 12 ++++++------ Library/Homebrew/descriptions.rb | 11 +++++++---- Library/Homebrew/search.rb | 13 ++++++------- Library/Homebrew/test/cask/cli/search_spec.rb | 3 +++ Library/Homebrew/test/search_spec.rb | 18 ++---------------- 7 files changed, 32 insertions(+), 40 deletions(-) diff --git a/Library/Homebrew/cmd/desc.rb b/Library/Homebrew/cmd/desc.rb index b704d08b85..1a75371a02 100644 --- a/Library/Homebrew/cmd/desc.rb +++ b/Library/Homebrew/cmd/desc.rb @@ -30,9 +30,10 @@ module Homebrew results.print elsif search_type.size > 1 odie "Pick one, and only one, of -s/--search, -n/--name, or -d/--description." - elsif arg = ARGV.named.join(" ") - regex = query_regexp(arg) - results = Descriptions.search(regex, search_type.first) + elsif !ARGV.named.empty? + arg = ARGV.named.join(" ") + string_or_regex = query_regexp(arg) + results = Descriptions.search(string_or_regex, search_type.first) results.print else odie "You must provide a search term." diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 4b699474e1..5d2385b74d 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -68,14 +68,16 @@ #: creating patches to the software. require "missing_formula" -require "cmd/search" require "formula_installer" require "development_tools" require "install" +require "search" module Homebrew module_function + extend Search + def install raise FormulaUnspecifiedError if ARGV.named.empty? @@ -261,10 +263,8 @@ module Homebrew return end - regex = query_regexp(e.name) - ohai "Searching for similarly named formulae..." - formulae_search_results = search_formulae(regex) + formulae_search_results = search_formulae(e.name) case formulae_search_results.length when 0 ofail "No similarly named formulae found." diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 4290b077e8..6d8ffe169c 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -55,9 +55,9 @@ module Homebrew if args.remaining.empty? puts Formatter.columns(Formula.full_names.sort) elsif args.desc? - query = args.remaining.first - regex = query_regexp(query) - Descriptions.search(regex, :desc).print + query = args.remaining.join(" ") + string_or_regex = query_regexp(query) + Descriptions.search(string_or_regex, :desc).print elsif args.remaining.first =~ HOMEBREW_TAP_FORMULA_REGEX query = args.remaining.first @@ -71,9 +71,9 @@ module Homebrew puts Formatter.columns(results) unless results.empty? else - query = args.remaining.first - regex = query_regexp(query) - local_results = search_formulae(regex) + query = args.remaining.join(" ") + string_or_regex = query_regexp(query) + local_results = search_formulae(string_or_regex) puts Formatter.columns(local_results.sort) unless local_results.empty? tap_results = search_taps(query).flatten.sort diff --git a/Library/Homebrew/descriptions.rb b/Library/Homebrew/descriptions.rb index 9d7c16b636..b14833e3d0 100644 --- a/Library/Homebrew/descriptions.rb +++ b/Library/Homebrew/descriptions.rb @@ -1,6 +1,7 @@ require "formula" require "formula_versions" require "search" +require "searchable" class Descriptions extend Homebrew::Search @@ -97,16 +98,18 @@ class Descriptions end # Given a regex, find all formulae whose specified fields contain a match. - def self.search(regex, field = :either) + def self.search(string_or_regex, field = :either) ensure_cache + @cache.extend(Searchable) + results = case field when :name - @cache.select { |name, _| simplify_string(name).match?(regex) } + @cache.search(string_or_regex) { |name, _| name } when :desc - @cache.select { |_, desc| simplify_string(desc).match?(regex) } + @cache.search(string_or_regex) { |_, desc| desc } when :either - @cache.select { |name, desc| simplify_string(name).match?(regex) || simplify_string(desc).match?(regex) } + @cache.search(string_or_regex) end new(results) diff --git a/Library/Homebrew/search.rb b/Library/Homebrew/search.rb index c94a5a61f8..6b97e1ad87 100644 --- a/Library/Homebrew/search.rb +++ b/Library/Homebrew/search.rb @@ -1,14 +1,12 @@ +require "searchable" + module Homebrew module Search - def simplify_string(string) - string.downcase.gsub(/[^a-z\d]/i, "") - end - def query_regexp(query) if m = query.match(%r{^/(.*)/$}) Regexp.new(m[1]) else - Regexp.new(simplify_string(query), Regexp::IGNORECASE) + query end rescue RegexpError raise "#{query} is not a valid regex." @@ -47,13 +45,14 @@ module Homebrew [[], []] end - def search_formulae(regex) + def search_formulae(string_or_regex) # Use stderr to avoid breaking parsed output $stderr.puts Formatter.headline("Searching local taps...", color: :blue) aliases = Formula.alias_full_names results = (Formula.full_names + aliases) - .select { |name| simplify_string(name).match?(regex) } + .extend(Searchable) + .search(string_or_regex) .sort results.map do |name| diff --git a/Library/Homebrew/test/cask/cli/search_spec.rb b/Library/Homebrew/test/cask/cli/search_spec.rb index 5500500b81..57e1206a0f 100644 --- a/Library/Homebrew/test/cask/cli/search_spec.rb +++ b/Library/Homebrew/test/cask/cli/search_spec.rb @@ -3,6 +3,7 @@ require_relative "shared_examples/invalid_option" describe Hbc::CLI::Search, :cask do before do allow(Tty).to receive(:width).and_return(0) + ENV.delete("HOMEBREW_NO_GITHUB_API") end it_behaves_like "a command that handles invalid options" @@ -51,6 +52,8 @@ describe Hbc::CLI::Search, :cask do end it "doesn't output anything to non-TTY stdout when there are no matches" do + allow(GitHub).to receive(:search_code).and_return([]) + expect { Hbc::CLI::Search.run("foo-bar-baz") } .to not_to_output.to_stdout .and not_to_output.to_stderr diff --git a/Library/Homebrew/test/search_spec.rb b/Library/Homebrew/test/search_spec.rb index 222ace3080..b64c4151b6 100644 --- a/Library/Homebrew/test/search_spec.rb +++ b/Library/Homebrew/test/search_spec.rb @@ -49,27 +49,13 @@ describe Homebrew::Search do end end - describe "#simplify_string" do - it "simplifies a query with dashes" do - expect(mod.query_regexp("que-ry")).to eq(/query/i) - end - - it "simplifies a query with @ symbols" do - expect(mod.query_regexp("query@1")).to eq(/query1/i) - end - end - describe "#query_regexp" do it "correctly parses a regex query" do expect(mod.query_regexp("/^query$/")).to eq(/^query$/) end - it "correctly converts a query string to a regex" do - expect(mod.query_regexp("query")).to eq(/query/i) - end - - it "simplifies a query with special symbols" do - expect(mod.query_regexp("que-ry")).to eq(/query/i) + it "returns the original string if it is not a regex query" do + expect(mod.query_regexp("query")).to eq("query") end it "raises an error if the query is an invalid regex" do From 0dfad30fb1c67945b4833ae48e9ba926156da543 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 5 Jun 2018 22:16:13 +0200 Subject: [PATCH 14/16] Only suggests formulae in `brew install`. --- Library/Homebrew/cmd/install.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 5d2385b74d..cd409480ac 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -281,7 +281,7 @@ module Homebrew # Do not search taps if the formula name is qualified return if e.name.include?("/") ohai "Searching taps..." - taps_search_results = search_taps(e.name) + taps_search_results, = search_taps(e.name) case taps_search_results.length when 0 ofail "No formulae found in taps." From 845cb99e29b7f37573eb9e499b1aed16ee6e7149 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 6 Jun 2018 13:13:45 +0200 Subject: [PATCH 15/16] Ignore `HOMEBREW_NO_GITHUB_API` when testing. --- Library/Homebrew/dev-cmd/tests.rb | 8 ++------ Library/Homebrew/test/cask/cli/search_spec.rb | 1 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/dev-cmd/tests.rb b/Library/Homebrew/dev-cmd/tests.rb index 8eabbf4558..183f67d806 100644 --- a/Library/Homebrew/dev-cmd/tests.rb +++ b/Library/Homebrew/dev-cmd/tests.rb @@ -39,16 +39,12 @@ module Homebrew ENV.delete("HOMEBREW_CASK_OPTS") ENV.delete("HOMEBREW_TEMP") ENV.delete("HOMEBREW_LINKAGE_CACHE") + ENV.delete("HOMEBREW_NO_GITHUB_API") ENV["HOMEBREW_NO_ANALYTICS_THIS_RUN"] = "1" ENV["HOMEBREW_DEVELOPER"] = "1" ENV["HOMEBREW_NO_COMPAT"] = "1" if args.no_compat? ENV["HOMEBREW_TEST_GENERIC_OS"] = "1" if args.generic? - - if args.online? - ENV["HOMEBREW_TEST_ONLINE"] = "1" - else - ENV["HOMEBREW_NO_GITHUB_API"] = "1" - end + ENV["HOMEBREW_TEST_ONLINE"] = "1" if args.online? if args.coverage? ENV["HOMEBREW_TESTS_COVERAGE"] = "1" diff --git a/Library/Homebrew/test/cask/cli/search_spec.rb b/Library/Homebrew/test/cask/cli/search_spec.rb index 57e1206a0f..379a27104d 100644 --- a/Library/Homebrew/test/cask/cli/search_spec.rb +++ b/Library/Homebrew/test/cask/cli/search_spec.rb @@ -3,7 +3,6 @@ require_relative "shared_examples/invalid_option" describe Hbc::CLI::Search, :cask do before do allow(Tty).to receive(:width).and_return(0) - ENV.delete("HOMEBREW_NO_GITHUB_API") end it_behaves_like "a command that handles invalid options" From cc93997fb7a0774faeba23ca1f0a881672b834ca Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 6 Jun 2018 13:24:55 +0200 Subject: [PATCH 16/16] Refactor `search_taps`. --- Library/Homebrew/cask/lib/hbc/cli/search.rb | 2 +- Library/Homebrew/cmd/install.rb | 2 +- Library/Homebrew/cmd/search.rb | 11 +++--- Library/Homebrew/search.rb | 38 ++++++++++++--------- Library/Homebrew/test/search_spec.rb | 6 ++-- 5 files changed, 33 insertions(+), 26 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/search.rb b/Library/Homebrew/cask/lib/hbc/cli/search.rb index addc35c98c..bda5374d53 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/search.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/search.rb @@ -36,7 +36,7 @@ module Hbc partial_matches = simplified_tokens.grep(/#{simplified_search_term}/i) { |t| all_tokens[simplified_tokens.index(t)] } end - _, remote_matches = search_taps(search_term, silent: true) + remote_matches = search_taps(search_term, silent: true)[:casks] [partial_matches, remote_matches, search_term] end diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index cd409480ac..fc1f56fc0a 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -281,7 +281,7 @@ module Homebrew # Do not search taps if the formula name is qualified return if e.name.include?("/") ohai "Searching taps..." - taps_search_results, = search_taps(e.name) + taps_search_results = search_taps(e.name)[:formulae] case taps_search_results.length when 0 ofail "No formulae found in taps." diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 6d8ffe169c..dd45fe63ad 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -61,12 +61,12 @@ module Homebrew elsif args.remaining.first =~ HOMEBREW_TAP_FORMULA_REGEX query = args.remaining.first - begin - result = Formulary.factory(query).name - results = Array(result) + results = begin + [Formulary.factory(query).name] rescue FormulaUnavailableError _, _, name = query.split("/", 3) - results = search_taps(name).flatten.sort + remote_results = search_taps(name) + [*remote_results[:formulae], *remote_results[:casks]].sort end puts Formatter.columns(results) unless results.empty? @@ -76,7 +76,8 @@ module Homebrew local_results = search_formulae(string_or_regex) puts Formatter.columns(local_results.sort) unless local_results.empty? - tap_results = search_taps(query).flatten.sort + remote_results = search_taps(query) + tap_results = [*remote_results[:formulae], *remote_results[:casks]].sort puts Formatter.columns(tap_results) unless tap_results.empty? if $stdout.tty? diff --git a/Library/Homebrew/search.rb b/Library/Homebrew/search.rb index 6b97e1ad87..bbabc2120d 100644 --- a/Library/Homebrew/search.rb +++ b/Library/Homebrew/search.rb @@ -13,36 +13,42 @@ module Homebrew end def search_taps(query, silent: false) - return [], [] if ENV["HOMEBREW_NO_GITHUB_API"] + results = { formulae: [], casks: [] } + + return results if ENV["HOMEBREW_NO_GITHUB_API"] unless silent # Use stderr to avoid breaking parsed output $stderr.puts Formatter.headline("Searching taps on GitHub...", color: :blue) end - matches = GitHub.search_code( - user: "Homebrew", - path: ["Formula", "Casks", "."], - filename: query, - extension: "rb", - ) + matches = begin + GitHub.search_code( + user: "Homebrew", + path: ["Formula", "Casks", "."], + filename: query, + extension: "rb", + ) + rescue GitHub::Error => error + opoo "Error searching on GitHub: #{error}\n" + return results + end - matches.inject([[], []]) do |(formulae, casks), match| + matches.each do |match| name = File.basename(match["path"], ".rb") tap = Tap.fetch(match["repository"]["full_name"]) full_name = "#{tap.name}/#{name}" - if tap.installed? && !match["path"].start_with?("Casks/") - [formulae, casks] - elsif match["path"].start_with?("Casks/") - [formulae, [*casks, full_name].sort] + next if tap.installed? && !match["path"].start_with?("Casks/") + + if match["path"].start_with?("Casks/") + results[:casks] = [*results[:casks], full_name].sort else - [[*formulae, full_name].sort, casks] + results[:formulae] = [*results[:formulae], full_name].sort end end - rescue GitHub::Error => error - opoo "Error searching on GitHub: #{error}\n" - [[], []] + + results end def search_formulae(string_or_regex) diff --git a/Library/Homebrew/test/search_spec.rb b/Library/Homebrew/test/search_spec.rb index b64c4151b6..1afcb1314a 100644 --- a/Library/Homebrew/test/search_spec.rb +++ b/Library/Homebrew/test/search_spec.rb @@ -14,14 +14,14 @@ describe Homebrew::Search do it "does not raise if `HOMEBREW_NO_GITHUB_API` is set" do ENV["HOMEBREW_NO_GITHUB_API"] = "1" - expect(mod.search_taps("some-formula")).to match([[], []]) + expect(mod.search_taps("some-formula")).to match(formulae: [], casks: []) end it "does not raise if the network fails" do allow(GitHub).to receive(:open_api).and_raise(GitHub::Error) expect(mod.search_taps("some-formula")) - .to match([[], []]) + .to match(formulae: [], casks: []) end it "returns Formulae and Casks separately" do @@ -45,7 +45,7 @@ describe Homebrew::Search do allow(GitHub).to receive(:open_api).and_yield(json_response) expect(mod.search_taps("some-formula")) - .to match([["homebrew/foo/some-formula"], ["homebrew/bar/some-cask"]]) + .to match(formulae: ["homebrew/foo/some-formula"], casks: ["homebrew/bar/some-cask"]) end end