From 234267cc937dbd59ef1219537f5bd07e24ef4c7f Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 8 Jan 2021 11:10:24 -0500 Subject: [PATCH 1/8] completions: make opt-in only --- Library/Homebrew/cmd/completions.rb | 52 +++++++++++++++++++++++++ Library/Homebrew/cmd/update-report.rb | 13 ++++++- Library/Homebrew/completions.rb | 53 ++++++++++++++++++++++++++ Library/Homebrew/utils/link.rb | 7 ++++ completions/internal_commands_list.txt | 1 + docs/Manpage.md | 11 ++++++ manpages/brew.1 | 11 ++++++ 7 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 Library/Homebrew/cmd/completions.rb create mode 100644 Library/Homebrew/completions.rb diff --git a/Library/Homebrew/cmd/completions.rb b/Library/Homebrew/cmd/completions.rb new file mode 100644 index 0000000000..8839aff511 --- /dev/null +++ b/Library/Homebrew/cmd/completions.rb @@ -0,0 +1,52 @@ +# typed: true +# frozen_string_literal: true + +require "cli/parser" +require "completions" + +module Homebrew + extend T::Sig + + module_function + + sig { returns(CLI::Parser) } + def completions_args + Homebrew::CLI::Parser.new do + usage_banner <<~EOS + `completions` [] + + Control whether Homebrew automatically links shell files. + Read more at . + + `brew completions` [`state`]: + Display the current state of Homebrew's completions. + + `brew completions` (`link`|`unlink`): + Link or unlink Homebrew's completions. + EOS + + max_named 1 + end + end + + def completions + args = completions_args.parse + + case args.named.first + when nil, "state" + if Completions.link_completions? + puts "Completions are not linked." + else + puts "Completions are linked." + end + when "link" + Completions.link! + puts "Completions are now linked." + when "unlink" + Completions.unlink! + puts "Completions are no longer linked." + else + raise UsageError, "unknown subcommand: #{args.named.first}" + end + end +end diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index f38b341394..4d7af92b36 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -8,6 +8,7 @@ require "descriptions" require "cleanup" require "description_cache_store" require "cli/parser" +require "completions" module Homebrew extend T::Sig @@ -150,6 +151,15 @@ module Homebrew puts "Already up-to-date." unless args.quiet? end + if Completions.read_completions_option.empty? + ohai "Homebrew completions are unlinked by default!" + puts <<~EOS + To opt-in to automatically linking Homebrew shell competion files, run: + brew completions link + Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} + EOS + end + Commands.rebuild_commands_completion_list link_completions_manpages_and_docs Tap.each(&:link_completions_and_manpages) @@ -188,7 +198,8 @@ module Homebrew def link_completions_manpages_and_docs(repository = HOMEBREW_REPOSITORY) command = "brew update" - Utils::Link.link_completions(repository, command) + + Completions.link_if_allowed! command: command Utils::Link.link_manpages(repository, command) Utils::Link.link_docs(repository, command) rescue => e diff --git a/Library/Homebrew/completions.rb b/Library/Homebrew/completions.rb new file mode 100644 index 0000000000..d266d95fa9 --- /dev/null +++ b/Library/Homebrew/completions.rb @@ -0,0 +1,53 @@ +# typed: true +# frozen_string_literal: true + +require "utils/link" + +# Helper functions for generating shell completions. +# +# @api private +module Completions + extend T::Sig + + module_function + + sig { params(command: String).void } + def link_if_allowed!(command: "brew completions link") + if link_completions? + link! command: command + else + unlink! + end + end + + sig { params(command: String).void } + def link!(command: "brew completions link") + write_completions_option "yes" + Utils::Link.link_completions HOMEBREW_REPOSITORY, command + end + + sig { void } + def unlink! + write_completions_option "no" + Utils::Link.unlink_completions HOMEBREW_REPOSITORY + end + + sig { returns(T::Boolean) } + def link_completions? + read_completions_option == "yes" + end + + sig { returns(String) } + def read_completions_option + HOMEBREW_REPOSITORY.cd do + Utils.popen_read("git", "config", "--get", "homebrew.linkcompletions").chomp + end + end + + sig { params(state: String).void } + def write_completions_option(state) + HOMEBREW_REPOSITORY.cd do + T.unsafe(self).safe_system "git", "config", "--replace-all", "homebrew.linkcompletions", state.to_s + end + end +end diff --git a/Library/Homebrew/utils/link.rb b/Library/Homebrew/utils/link.rb index 8078390f60..8ca992a802 100644 --- a/Library/Homebrew/utils/link.rb +++ b/Library/Homebrew/utils/link.rb @@ -1,6 +1,8 @@ # typed: true # frozen_string_literal: true +require "completions" + module Utils # Helper functions for creating symlinks. # @@ -64,6 +66,11 @@ module Utils end def link_completions(path, command) + unless Completions.link_completions? + unlink_completions path + return + end + link_src_dst_dirs(path/"completions/bash", HOMEBREW_PREFIX/"etc/bash_completion.d", command) link_src_dst_dirs(path/"completions/zsh", HOMEBREW_PREFIX/"share/zsh/site-functions", command) link_src_dst_dirs(path/"completions/fish", HOMEBREW_PREFIX/"share/fish/vendor_completions.d", command) diff --git a/completions/internal_commands_list.txt b/completions/internal_commands_list.txt index f1a952a9e1..13e278cc73 100644 --- a/completions/internal_commands_list.txt +++ b/completions/internal_commands_list.txt @@ -25,6 +25,7 @@ cat cleanup command commands +completions config configure create diff --git a/docs/Manpage.md b/docs/Manpage.md index bda3f0502c..ec35d57916 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -94,6 +94,17 @@ Show lists of built-in and external commands. * `--include-aliases`: Include aliases of internal commands. +### `completions` [*`subcommand`*] + +Control whether Homebrew automatically links shell files. +Read more at . + +`brew completions` [`state`] +
Display the current state of Homebrew's completions. + +`brew completions` (`link`|`unlink`) +
Link or unlink Homebrew's completions. + ### `config` Show Homebrew and system configuration info useful for debugging. If you file diff --git a/manpages/brew.1 b/manpages/brew.1 index 09132afcc7..6edecb36bf 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -93,6 +93,17 @@ List only the names of commands without category headers\. \fB\-\-include\-aliases\fR Include aliases of internal commands\. . +.SS "\fBcompletions\fR [\fIsubcommand\fR]" +Control whether Homebrew automatically links shell files\. Read more at \fIhttps://docs\.brew\.sh/Shell\-Completion\fR\. +. +.P +\fBbrew completions\fR [\fBstate\fR] + Display the current state of Homebrew\'s completions\. +. +.P +\fBbrew completions\fR (\fBlink\fR|\fBunlink\fR) + Link or unlink Homebrew\'s completions\. +. .SS "\fBconfig\fR" Show Homebrew and system configuration info useful for debugging\. If you file a bug report, you will be required to provide this information\. . From 74787ca0eeb3a1d4c19420dced32c9f5965175b0 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 8 Jan 2021 11:26:33 -0500 Subject: [PATCH 2/8] docs: update shell completions instructions --- docs/Shell-Completion.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/Shell-Completion.md b/docs/Shell-Completion.md index bb5aa592f5..6dd1785bca 100644 --- a/docs/Shell-Completion.md +++ b/docs/Shell-Completion.md @@ -4,7 +4,9 @@ Homebrew comes with completion definitions for the `brew` command. Some packages `zsh`, `bash` and `fish` are currently supported. -You must configure your shell to enable its completion support. This is because the Homebrew-managed completions are stored under `HOMEBREW_PREFIX` which your system shell may not be aware of, and since it is difficult to automatically configure `bash` and `zsh` completions in a robust manner, the Homebrew installer does not do it for you. +Shell completions for built-in Homebrew commands are not automatically installed. To opt-in to using our completions, they need to be linked to `HOMEBREW_PREFIX` by running `brew completions link`. + +You must then configure your shell to enable its completion support. This is because the Homebrew-managed completions are stored under `HOMEBREW_PREFIX` which your system shell may not be aware of, and since it is difficult to automatically configure `bash` and `zsh` completions in a robust manner, the Homebrew installer does not do it for you. ## Configuring Completions in `bash` From e9d09c4d8f214329de12acb7d059c1fd37d1360c Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 8 Jan 2021 17:11:20 -0500 Subject: [PATCH 3/8] tests: add and fix tests --- Library/Homebrew/cmd/completions.rb | 4 +-- Library/Homebrew/test/.rubocop_todo.yml | 3 +- Library/Homebrew/test/cmd/completions_spec.rb | 28 ++++++++++++++++++ Library/Homebrew/test/tap_spec.rb | 29 ++++++++++++++++++- 4 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 Library/Homebrew/test/cmd/completions_spec.rb diff --git a/Library/Homebrew/cmd/completions.rb b/Library/Homebrew/cmd/completions.rb index 8839aff511..814c7500d7 100644 --- a/Library/Homebrew/cmd/completions.rb +++ b/Library/Homebrew/cmd/completions.rb @@ -35,9 +35,9 @@ module Homebrew case args.named.first when nil, "state" if Completions.link_completions? - puts "Completions are not linked." - else puts "Completions are linked." + else + puts "Completions are not linked." end when "link" Completions.link! diff --git a/Library/Homebrew/test/.rubocop_todo.yml b/Library/Homebrew/test/.rubocop_todo.yml index 7e7fa024f6..364b23edc8 100644 --- a/Library/Homebrew/test/.rubocop_todo.yml +++ b/Library/Homebrew/test/.rubocop_todo.yml @@ -22,7 +22,7 @@ RSpec/InstanceVariable: - 'utils/git_spec.rb' - 'version_spec.rb' -# Offense count: 76 +# Offense count: 81 RSpec/MultipleDescribes: Exclude: - 'ENV_spec.rb' @@ -37,6 +37,7 @@ RSpec/MultipleDescribes: - 'cmd/autoremove_spec.rb' - 'cmd/cleanup_spec.rb' - 'cmd/commands_spec.rb' + - 'cmd/completions_spec.rb' - 'cmd/config_spec.rb' - 'cmd/deps_spec.rb' - 'cmd/desc_spec.rb' diff --git a/Library/Homebrew/test/cmd/completions_spec.rb b/Library/Homebrew/test/cmd/completions_spec.rb new file mode 100644 index 0000000000..23f0365402 --- /dev/null +++ b/Library/Homebrew/test/cmd/completions_spec.rb @@ -0,0 +1,28 @@ +# typed: false +# frozen_string_literal: true + +require "cmd/shared_examples/args_parse" + +describe "Homebrew.completions_args" do + it_behaves_like "parseable arguments" +end + +describe "brew completions", :integration_test do + it "runs the status subcommand correctly" do + HOMEBREW_REPOSITORY.cd do + system "git", "init" + end + + brew "completions", "link" + expect { brew "completions" } + .to output(/Completions are linked/).to_stdout + .and not_to_output.to_stderr + .and be_a_success + + brew "completions", "unlink" + expect { brew "completions" } + .to output(/Completions are not linked/).to_stdout + .and not_to_output.to_stderr + .and be_a_success + end +end diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index 3a5ddf8197..0236f6a6f5 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -84,6 +84,13 @@ describe Tap do end end + def setup_completion(link:) + HOMEBREW_REPOSITORY.cd do + system "git", "init" + system "git", "config", "--replace-all", "homebrew.linkcompletions", link + end + end + specify "::fetch" do expect(described_class.fetch("Homebrew", "core")).to be_kind_of(CoreTap) expect(described_class.fetch("Homebrew", "homebrew")).to be_kind_of(CoreTap) @@ -285,6 +292,7 @@ describe Tap do specify "#install and #uninstall" do setup_tap_files setup_git_repo + setup_completion link: "yes" tap = described_class.new("Homebrew", "bar") @@ -308,9 +316,10 @@ describe Tap do (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? end - specify "#link_completions_and_manpages" do + specify "#link_completions_and_manpages when completions are enabled" do setup_tap_files setup_git_repo + setup_completion link: "yes" tap = described_class.new("Homebrew", "baz") tap.install clone_target: subject.path/".git" (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete @@ -328,6 +337,24 @@ describe Tap do (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? end + specify "#link_completions_and_manpages when completions are disabled" do + setup_tap_files + setup_git_repo + setup_completion link: "no" + tap = described_class.new("Homebrew", "baz") + tap.install clone_target: subject.path/".git" + (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete + tap.link_completions_and_manpages + expect(HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").to be_a_file + expect(HOMEBREW_PREFIX/"etc/bash_completion.d/brew-tap-cmd").not_to be_a_file + expect(HOMEBREW_PREFIX/"share/zsh/site-functions/_brew-tap-cmd").not_to be_a_file + expect(HOMEBREW_PREFIX/"share/fish/vendor_completions.d/brew-tap-cmd.fish").not_to be_a_file + tap.uninstall + ensure + (HOMEBREW_PREFIX/"etc").rmtree if (HOMEBREW_PREFIX/"etc").exist? + (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? + end + specify "#config" do setup_git_repo From bed16128cc73b9a18f1a5b5b26d4f30e095fdb29 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sun, 10 Jan 2021 16:32:18 -0500 Subject: [PATCH 4/8] completions: fix usage text --- Library/Homebrew/cmd/completions.rb | 2 +- docs/Manpage.md | 2 +- manpages/brew.1 | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cmd/completions.rb b/Library/Homebrew/cmd/completions.rb index 814c7500d7..6f0ba352da 100644 --- a/Library/Homebrew/cmd/completions.rb +++ b/Library/Homebrew/cmd/completions.rb @@ -15,7 +15,7 @@ module Homebrew usage_banner <<~EOS `completions` [] - Control whether Homebrew automatically links shell files. + Control whether Homebrew automatically links shell completion files. Read more at . `brew completions` [`state`]: diff --git a/docs/Manpage.md b/docs/Manpage.md index ec35d57916..17160e407f 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -96,7 +96,7 @@ Show lists of built-in and external commands. ### `completions` [*`subcommand`*] -Control whether Homebrew automatically links shell files. +Control whether Homebrew automatically links shell completion files. Read more at . `brew completions` [`state`] diff --git a/manpages/brew.1 b/manpages/brew.1 index 6edecb36bf..419c623655 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -94,7 +94,7 @@ List only the names of commands without category headers\. Include aliases of internal commands\. . .SS "\fBcompletions\fR [\fIsubcommand\fR]" -Control whether Homebrew automatically links shell files\. Read more at \fIhttps://docs\.brew\.sh/Shell\-Completion\fR\. +Control whether Homebrew automatically links shell completion files\. Read more at \fIhttps://docs\.brew\.sh/Shell\-Completion\fR\. . .P \fBbrew completions\fR [\fBstate\fR] From e7b369273a9b23f0674491fe33a8d3aaf197c292 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Mon, 11 Jan 2021 12:24:48 -0500 Subject: [PATCH 5/8] completions: link official taps automatically --- Library/Homebrew/cmd/update-report.rb | 13 +--- Library/Homebrew/completions.rb | 64 +++++++++++++------ Library/Homebrew/tap.rb | 9 ++- Library/Homebrew/test/cmd/completions_spec.rb | 6 -- Library/Homebrew/test/tap_spec.rb | 30 +++++++-- Library/Homebrew/utils/link.rb | 7 -- docs/Shell-Completion.md | 4 +- 7 files changed, 82 insertions(+), 51 deletions(-) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index 4d7af92b36..f38b341394 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -8,7 +8,6 @@ require "descriptions" require "cleanup" require "description_cache_store" require "cli/parser" -require "completions" module Homebrew extend T::Sig @@ -151,15 +150,6 @@ module Homebrew puts "Already up-to-date." unless args.quiet? end - if Completions.read_completions_option.empty? - ohai "Homebrew completions are unlinked by default!" - puts <<~EOS - To opt-in to automatically linking Homebrew shell competion files, run: - brew completions link - Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} - EOS - end - Commands.rebuild_commands_completion_list link_completions_manpages_and_docs Tap.each(&:link_completions_and_manpages) @@ -198,8 +188,7 @@ module Homebrew def link_completions_manpages_and_docs(repository = HOMEBREW_REPOSITORY) command = "brew update" - - Completions.link_if_allowed! command: command + Utils::Link.link_completions(repository, command) Utils::Link.link_manpages(repository, command) Utils::Link.link_docs(repository, command) rescue => e diff --git a/Library/Homebrew/completions.rb b/Library/Homebrew/completions.rb index d266d95fa9..9bde547f40 100644 --- a/Library/Homebrew/completions.rb +++ b/Library/Homebrew/completions.rb @@ -11,25 +11,22 @@ module Completions module_function - sig { params(command: String).void } - def link_if_allowed!(command: "brew completions link") - if link_completions? - link! command: command - else - unlink! - end - end - - sig { params(command: String).void } - def link!(command: "brew completions link") + sig { void } + def link! write_completions_option "yes" - Utils::Link.link_completions HOMEBREW_REPOSITORY, command + Tap.each do |tap| + Utils::Link.link_completions tap.path, "brew completions link" + end end sig { void } def unlink! write_completions_option "no" - Utils::Link.unlink_completions HOMEBREW_REPOSITORY + Tap.each do |tap| + next if tap.official? + + Utils::Link.unlink_completions tap.path + end end sig { returns(T::Boolean) } @@ -37,17 +34,46 @@ module Completions read_completions_option == "yes" end - sig { returns(String) } - def read_completions_option + sig { returns(T::Boolean) } + def completions_to_link? + shells = %w[bash fish zsh] + Tap.each do |tap| + next if tap.official? + + shells.each do |shell| + return true if (tap.path/"completions/#{shell}").exist? + end + end + + false + end + + sig { params(option: String).returns(String) } + def read_completions_option(option: "linkcompletions") HOMEBREW_REPOSITORY.cd do - Utils.popen_read("git", "config", "--get", "homebrew.linkcompletions").chomp + Utils.popen_read("git", "config", "--get", "homebrew.#{option}").chomp end end - sig { params(state: String).void } - def write_completions_option(state) + sig { params(state: String, option: String).void } + def write_completions_option(state, option: "linkcompletions") HOMEBREW_REPOSITORY.cd do - T.unsafe(self).safe_system "git", "config", "--replace-all", "homebrew.linkcompletions", state.to_s + T.unsafe(self).safe_system "git", "config", "--replace-all", "homebrew.#{option}", state.to_s end end + + sig { void } + def show_completions_message_if_needed + return if read_completions_option(option: "completionsmessageshown") == "yes" + return unless completions_to_link? + + T.unsafe(self).ohai "Homebrew completions for external commands are unlinked by default!" + T.unsafe(self).puts <<~EOS + To opt-in to automatically linking Homebrew shell competion files, run: + brew completions link + Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} + EOS + + write_completions_option("yes", option: "completionsmessageshown") + end end diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index da99c5ff0a..b8daea201a 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "commands" +require "completions" require "extend/cachable" require "description_cache_store" @@ -340,7 +341,13 @@ class Tap def link_completions_and_manpages command = "brew tap --repair" Utils::Link.link_manpages(path, command) - Utils::Link.link_completions(path, command) + + Completions.show_completions_message_if_needed + if official? || Completions.link_completions? + Utils::Link.link_completions(path, command) + else + Utils::Link.unlink_completions(path) + end end # Uninstall this {Tap}. diff --git a/Library/Homebrew/test/cmd/completions_spec.rb b/Library/Homebrew/test/cmd/completions_spec.rb index 23f0365402..0c093bec6f 100644 --- a/Library/Homebrew/test/cmd/completions_spec.rb +++ b/Library/Homebrew/test/cmd/completions_spec.rb @@ -18,11 +18,5 @@ describe "brew completions", :integration_test do .to output(/Completions are linked/).to_stdout .and not_to_output.to_stderr .and be_a_success - - brew "completions", "unlink" - expect { brew "completions" } - .to output(/Completions are not linked/).to_stdout - .and not_to_output.to_stderr - .and be_a_success end end diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index 0236f6a6f5..eb4a64e121 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -88,6 +88,7 @@ describe Tap do HOMEBREW_REPOSITORY.cd do system "git", "init" system "git", "config", "--replace-all", "homebrew.linkcompletions", link + system "git", "config", "--replace-all", "homebrew.completionsmessageshown", "yes" end end @@ -316,11 +317,11 @@ describe Tap do (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? end - specify "#link_completions_and_manpages when completions are enabled" do + specify "#link_completions_and_manpages when completions are enabled for non-official tap" do setup_tap_files setup_git_repo setup_completion link: "yes" - tap = described_class.new("Homebrew", "baz") + tap = described_class.new("NotHomebrew", "baz") tap.install clone_target: subject.path/".git" (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete (HOMEBREW_PREFIX/"etc/bash_completion.d/brew-tap-cmd").delete @@ -337,11 +338,11 @@ describe Tap do (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? end - specify "#link_completions_and_manpages when completions are disabled" do + specify "#link_completions_and_manpages when completions are disabled for non-official tap" do setup_tap_files setup_git_repo setup_completion link: "no" - tap = described_class.new("Homebrew", "baz") + tap = described_class.new("NotHomebrew", "baz") tap.install clone_target: subject.path/".git" (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete tap.link_completions_and_manpages @@ -355,6 +356,27 @@ describe Tap do (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? end + specify "#link_completions_and_manpages when completions are enabled for official tap" do + setup_tap_files + setup_git_repo + setup_completion link: "no" + tap = described_class.new("Homebrew", "baz") + tap.install clone_target: subject.path/".git" + (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete + (HOMEBREW_PREFIX/"etc/bash_completion.d/brew-tap-cmd").delete + (HOMEBREW_PREFIX/"share/zsh/site-functions/_brew-tap-cmd").delete + (HOMEBREW_PREFIX/"share/fish/vendor_completions.d/brew-tap-cmd.fish").delete + tap.link_completions_and_manpages + expect(HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").to be_a_file + expect(HOMEBREW_PREFIX/"etc/bash_completion.d/brew-tap-cmd").to be_a_file + expect(HOMEBREW_PREFIX/"share/zsh/site-functions/_brew-tap-cmd").to be_a_file + expect(HOMEBREW_PREFIX/"share/fish/vendor_completions.d/brew-tap-cmd.fish").to be_a_file + tap.uninstall + ensure + (HOMEBREW_PREFIX/"etc").rmtree if (HOMEBREW_PREFIX/"etc").exist? + (HOMEBREW_PREFIX/"share").rmtree if (HOMEBREW_PREFIX/"share").exist? + end + specify "#config" do setup_git_repo diff --git a/Library/Homebrew/utils/link.rb b/Library/Homebrew/utils/link.rb index 8ca992a802..8078390f60 100644 --- a/Library/Homebrew/utils/link.rb +++ b/Library/Homebrew/utils/link.rb @@ -1,8 +1,6 @@ # typed: true # frozen_string_literal: true -require "completions" - module Utils # Helper functions for creating symlinks. # @@ -66,11 +64,6 @@ module Utils end def link_completions(path, command) - unless Completions.link_completions? - unlink_completions path - return - end - link_src_dst_dirs(path/"completions/bash", HOMEBREW_PREFIX/"etc/bash_completion.d", command) link_src_dst_dirs(path/"completions/zsh", HOMEBREW_PREFIX/"share/zsh/site-functions", command) link_src_dst_dirs(path/"completions/fish", HOMEBREW_PREFIX/"share/fish/vendor_completions.d", command) diff --git a/docs/Shell-Completion.md b/docs/Shell-Completion.md index 6dd1785bca..b68d23744b 100644 --- a/docs/Shell-Completion.md +++ b/docs/Shell-Completion.md @@ -4,10 +4,10 @@ Homebrew comes with completion definitions for the `brew` command. Some packages `zsh`, `bash` and `fish` are currently supported. -Shell completions for built-in Homebrew commands are not automatically installed. To opt-in to using our completions, they need to be linked to `HOMEBREW_PREFIX` by running `brew completions link`. - You must then configure your shell to enable its completion support. This is because the Homebrew-managed completions are stored under `HOMEBREW_PREFIX` which your system shell may not be aware of, and since it is difficult to automatically configure `bash` and `zsh` completions in a robust manner, the Homebrew installer does not do it for you. +Shell completions for external Homebrew commands are not automatically installed. To opt-in to using completions for external commands (if provided), they need to be linked to `HOMEBREW_PREFIX` by running `brew completions link`. + ## Configuring Completions in `bash` To make Homebrew's completions available in `bash`, you must source the definitions as part of your shell's startup. Add the following to your `~/.bash_profile` (or, if it doesn't exist, `~/.profile`): From f1f3fdc315238e6f86892f6228f7da5c5eb462a3 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Tue, 12 Jan 2021 16:27:25 -0500 Subject: [PATCH 6/8] settings: add module for managing git config settings --- Library/Homebrew/brew.rb | 6 +- Library/Homebrew/cmd/update-report.rb | 24 ++-- Library/Homebrew/completions.rb | 25 +--- Library/Homebrew/settings.rb | 39 ++++++ Library/Homebrew/tap.rb | 9 +- Library/Homebrew/test/completions_spec.rb | 143 ++++++++++++++++++++++ Library/Homebrew/test/settings_spec.rb | 74 +++++++++++ Library/Homebrew/test/tap_spec.rb | 12 +- Library/Homebrew/utils/analytics.rb | 33 ++--- 9 files changed, 289 insertions(+), 76 deletions(-) create mode 100644 Library/Homebrew/settings.rb create mode 100644 Library/Homebrew/test/completions_spec.rb create mode 100644 Library/Homebrew/test/settings_spec.rb diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index ca712613e4..d234adabc7 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -85,16 +85,14 @@ begin ENV["PATH"] = path require "commands" + require "settings" if cmd internal_cmd = Commands.valid_internal_cmd?(cmd) internal_cmd ||= begin internal_dev_cmd = Commands.valid_internal_dev_cmd?(cmd) if internal_dev_cmd && !Homebrew::EnvConfig.developer? - if (HOMEBREW_REPOSITORY/".git/config").exist? - system "git", "config", "--file=#{HOMEBREW_REPOSITORY}/.git/config", - "--replace-all", "homebrew.devcmdrun", "true" - end + Settings.write "devcmdrun", true ENV["HOMEBREW_DEV_CMD_RUN"] = "1" end internal_dev_cmd diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index f38b341394..3a91124812 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -8,6 +8,7 @@ require "descriptions" require "cleanup" require "description_cache_store" require "cli/parser" +require "settings" module Homebrew extend T::Sig @@ -63,16 +64,12 @@ module Homebrew Utils::Analytics.messages_displayed! if $stdout.tty? end - HOMEBREW_REPOSITORY.cd do - donation_message_displayed = - Utils.popen_read("git", "config", "--get", "homebrew.donationmessage").chomp == "true" - if !donation_message_displayed && !args.quiet? - ohai "Homebrew is run entirely by unpaid volunteers. Please consider donating:" - puts " #{Formatter.url("https://github.com/Homebrew/brew#donations")}\n" + if Settings.read("donationmessage") != "true" && !args.quiet? + ohai "Homebrew is run entirely by unpaid volunteers. Please consider donating:" + puts " #{Formatter.url("https://github.com/Homebrew/brew#donations")}\n" - # Consider the message possibly missed if not a TTY. - safe_system "git", "config", "--replace-all", "homebrew.donationmessage", "true" if $stdout.tty? - end + # Consider the message possibly missed if not a TTY. + Settings.write "donationmessage", true if $stdout.tty? end install_core_tap_if_necessary @@ -89,19 +86,14 @@ module Homebrew puts "Updated Homebrew from #{shorten_revision(initial_revision)} to #{shorten_revision(current_revision)}." updated = true - old_tag = if (HOMEBREW_REPOSITORY/".git/config").exist? - Utils.popen_read( - "git", "config", "--file=#{HOMEBREW_REPOSITORY}/.git/config", "--get", "homebrew.latesttag" - ).chomp.presence - end + old_tag = Settings.read "latesttag" new_tag = Utils.popen_read( "git", "-C", HOMEBREW_REPOSITORY, "tag", "--list", "--sort=-version:refname", "*.*" ).lines.first.chomp if new_tag != old_tag - system "git", "config", "--file=#{HOMEBREW_REPOSITORY}/.git/config", - "--replace-all", "homebrew.latesttag", new_tag + Settings.write "latesttag", new_tag new_repository_version = new_tag end end diff --git a/Library/Homebrew/completions.rb b/Library/Homebrew/completions.rb index 9bde547f40..356a9dbabd 100644 --- a/Library/Homebrew/completions.rb +++ b/Library/Homebrew/completions.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "utils/link" +require "settings" # Helper functions for generating shell completions. # @@ -13,7 +14,7 @@ module Completions sig { void } def link! - write_completions_option "yes" + Settings.write :linkcompletions, true Tap.each do |tap| Utils::Link.link_completions tap.path, "brew completions link" end @@ -21,7 +22,7 @@ module Completions sig { void } def unlink! - write_completions_option "no" + Settings.write :linkcompletions, false Tap.each do |tap| next if tap.official? @@ -31,7 +32,7 @@ module Completions sig { returns(T::Boolean) } def link_completions? - read_completions_option == "yes" + Settings.read(:linkcompletions) == "true" end sig { returns(T::Boolean) } @@ -48,23 +49,9 @@ module Completions false end - sig { params(option: String).returns(String) } - def read_completions_option(option: "linkcompletions") - HOMEBREW_REPOSITORY.cd do - Utils.popen_read("git", "config", "--get", "homebrew.#{option}").chomp - end - end - - sig { params(state: String, option: String).void } - def write_completions_option(state, option: "linkcompletions") - HOMEBREW_REPOSITORY.cd do - T.unsafe(self).safe_system "git", "config", "--replace-all", "homebrew.#{option}", state.to_s - end - end - sig { void } def show_completions_message_if_needed - return if read_completions_option(option: "completionsmessageshown") == "yes" + return if Settings.read(:completionsmessageshown) == "true" return unless completions_to_link? T.unsafe(self).ohai "Homebrew completions for external commands are unlinked by default!" @@ -74,6 +61,6 @@ module Completions Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} EOS - write_completions_option("yes", option: "completionsmessageshown") + Settings.write :completionsmessageshown, true end end diff --git a/Library/Homebrew/settings.rb b/Library/Homebrew/settings.rb new file mode 100644 index 0000000000..24286026ac --- /dev/null +++ b/Library/Homebrew/settings.rb @@ -0,0 +1,39 @@ +# typed: true +# frozen_string_literal: true + +# Helper functions for reading and writing settings. +# +# @api private +module Settings + extend T::Sig + + module_function + + sig { params(setting: T.any(String, Symbol), repo: Pathname).returns(T.nilable(String)) } + def read(setting, repo: HOMEBREW_REPOSITORY) + return unless (repo/".git/config").exist? + + repo.cd do + Utils.popen_read("git", "config", "--get", "homebrew.#{setting}").chomp.presence + end + end + + sig { params(setting: T.any(String, Symbol), value: T.any(String, T::Boolean), repo: Pathname).void } + def write(setting, value, repo: HOMEBREW_REPOSITORY) + return unless (repo/".git/config").exist? + + repo.cd do + T.unsafe(self).safe_system "git", "config", "--replace-all", "homebrew.#{setting}", value.to_s + end + end + + sig { params(setting: T.any(String, Symbol), repo: Pathname).void } + def delete(setting, repo: HOMEBREW_REPOSITORY) + return unless (repo/".git/config").exist? + return if read(setting, repo: repo).blank? + + repo.cd do + T.unsafe(self).safe_system "git", "config", "--unset-all", "homebrew.#{setting}" + end + end +end diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index b8daea201a..b9e90b1686 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -5,6 +5,7 @@ require "commands" require "completions" require "extend/cachable" require "description_cache_store" +require "settings" # A {Tap} is used to extend the formulae provided by Homebrew core. # Usually, it's synced with a remote Git repository. And it's likely @@ -821,18 +822,14 @@ class TapConfig return unless tap.git? return unless Utils::Git.available? - tap.path.cd do - Utils.popen_read("git", "config", "--get", "homebrew.#{key}").chomp.presence - end + Settings.read key, repo: tap.path end def []=(key, value) return unless tap.git? return unless Utils::Git.available? - tap.path.cd do - safe_system "git", "config", "--replace-all", "homebrew.#{key}", value.to_s - end + Settings.write key, value.to_s, repo: tap.path end end diff --git a/Library/Homebrew/test/completions_spec.rb b/Library/Homebrew/test/completions_spec.rb new file mode 100644 index 0000000000..41c9f95ef0 --- /dev/null +++ b/Library/Homebrew/test/completions_spec.rb @@ -0,0 +1,143 @@ +# typed: false +# frozen_string_literal: true + +require "completions" + +describe Completions do + let(:internal_path) { HOMEBREW_REPOSITORY/"Library/Taps/homebrew/homebrew-bar" } + let(:external_path) { HOMEBREW_REPOSITORY/"Library/Taps/foo/homebrew-bar" } + + before do + HOMEBREW_REPOSITORY.cd do + system "git", "init" + end + internal_path.mkpath + external_path.mkpath + end + + def setup_completions(external:) + (internal_path/"completions/bash/foo_internal").write "#foo completions" + if external + (external_path/"completions/bash/foo_external").write "#foo completions" + elsif (external_path/"completions/bash/foo_external").exist? + (external_path/"completions/bash/foo_external").delete + end + end + + def setup_completions_setting(state, setting: "linkcompletions") + HOMEBREW_REPOSITORY.cd do + system "git", "config", "--replace-all", "homebrew.#{setting}", state.to_s + end + end + + def read_completions_setting(setting: "linkcompletions") + HOMEBREW_REPOSITORY.cd do + Utils.popen_read("git", "config", "--get", "homebrew.#{setting}").chomp.presence + end + end + + def delete_completions_setting(setting: "linkcompletions") + HOMEBREW_REPOSITORY.cd do + system "git", "config", "--unset-all", "homebrew.#{setting}" + end + end + + after do + FileUtils.rm_rf internal_path + FileUtils.rm_rf external_path.dirname + end + + describe ".link!" do + it "sets homebrew.linkcompletions to true" do + setup_completions_setting false + expect { described_class.link! }.not_to raise_error + expect(read_completions_setting).to eq "true" + end + + it "sets homebrew.linkcompletions to true if unset" do + delete_completions_setting + expect { described_class.link! }.not_to raise_error + expect(read_completions_setting).to eq "true" + end + + it "keeps homebrew.linkcompletions set to true" do + setup_completions_setting true + expect { described_class.link! }.not_to raise_error + expect(read_completions_setting).to eq "true" + end + end + + describe ".unlink!" do + it "sets homebrew.linkcompletions to false" do + setup_completions_setting true + expect { described_class.unlink! }.not_to raise_error + expect(read_completions_setting).to eq "false" + end + + it "sets homebrew.linkcompletions to false if unset" do + delete_completions_setting + expect { described_class.unlink! }.not_to raise_error + expect(read_completions_setting).to eq "false" + end + + it "keeps homebrew.linkcompletions set to false" do + setup_completions_setting false + expect { described_class.unlink! }.not_to raise_error + expect(read_completions_setting).to eq "false" + end + end + + describe ".link_completions?" do + it "returns true if homebrew.linkcompletions is true" do + setup_completions_setting true + expect(described_class.link_completions?).to be true + end + + it "returns false if homebrew.linkcompletions is false" do + setup_completions_setting false + expect(described_class.link_completions?).to be false + end + + it "returns false if homebrew.linkcompletions is not set" do + expect(described_class.link_completions?).to be false + end + end + + describe ".completions_to_link?" do + it "returns false if only internal taps have completions" do + setup_completions external: false + expect(described_class.completions_to_link?).to be false + end + + it "returns true if external taps have completions" do + setup_completions external: true + expect(described_class.completions_to_link?).to be true + end + end + + describe ".show_completions_message_if_needed" do + it "doesn't show the message if there are no completions to link" do + setup_completions external: false + delete_completions_setting setting: :completionsmessageshown + expect { described_class.show_completions_message_if_needed }.not_to output.to_stdout + end + + it "doesn't show the message if there are completions to link but the message has already been shown" do + setup_completions external: true + setup_completions_setting true, setting: :completionsmessageshown + expect { described_class.show_completions_message_if_needed }.not_to output.to_stdout + end + + it "shows the message if there are completions to link and the message hasn't already been shown" do + setup_completions external: true + delete_completions_setting setting: :completionsmessageshown + + # This will fail because the method calls `puts`. + # If we output the `ohai` andcatch the error, we can be usre that the message is showing. + error_message = "private method `puts' called for Completions:Module" + expect { described_class.show_completions_message_if_needed } + .to output.to_stdout + .and raise_error(NoMethodError, error_message) + end + end +end diff --git a/Library/Homebrew/test/settings_spec.rb b/Library/Homebrew/test/settings_spec.rb new file mode 100644 index 0000000000..1c42f7fe76 --- /dev/null +++ b/Library/Homebrew/test/settings_spec.rb @@ -0,0 +1,74 @@ +# typed: false +# frozen_string_literal: true + +require "settings" + +describe Settings do + before do + HOMEBREW_REPOSITORY.cd do + system "git", "init" + end + end + + def setup_setting + HOMEBREW_REPOSITORY.cd do + system "git", "config", "--replace-all", "homebrew.foo", "true" + end + end + + describe ".read" do + it "returns the correct value for a setting" do + setup_setting + expect(described_class.read("foo")).to eq "true" + end + + it "returns the correct value for a setting as a symbol" do + setup_setting + expect(described_class.read(:foo)).to eq "true" + end + + it "returns nil when setting is not set" do + setup_setting + expect(described_class.read("bar")).to be_nil + end + + it "runs on a repo without a configuration file" do + expect { described_class.read("foo", repo: HOMEBREW_REPOSITORY/"bar") }.not_to raise_error + end + end + + describe ".write" do + it "writes over an existing value" do + setup_setting + described_class.write :foo, false + expect(described_class.read("foo")).to eq "false" + end + + it "writes a new value" do + setup_setting + described_class.write :bar, "abcde" + expect(described_class.read("bar")).to eq "abcde" + end + + it "returns if the repo doesn't have a configuration file" do + expect { described_class.write("foo", repo: HOMEBREW_REPOSITORY/"bar") }.not_to raise_error + end + end + + describe ".delete" do + it "deletes an existing setting" do + setup_setting + described_class.delete(:foo) + expect(described_class.read("foo")).to be_nil + end + + it "deletes a non-existing setting" do + setup_setting + expect { described_class.delete(:bar) }.not_to raise_error + end + + it "returns if the repo doesn't have a configuration file" do + expect { described_class.delete("foo", repo: HOMEBREW_REPOSITORY/"bar") }.not_to raise_error + end + end +end diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index eb4a64e121..691e5fa61c 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -87,8 +87,8 @@ describe Tap do def setup_completion(link:) HOMEBREW_REPOSITORY.cd do system "git", "init" - system "git", "config", "--replace-all", "homebrew.linkcompletions", link - system "git", "config", "--replace-all", "homebrew.completionsmessageshown", "yes" + system "git", "config", "--replace-all", "homebrew.linkcompletions", link.to_s + system "git", "config", "--replace-all", "homebrew.completionsmessageshown", "true" end end @@ -293,7 +293,7 @@ describe Tap do specify "#install and #uninstall" do setup_tap_files setup_git_repo - setup_completion link: "yes" + setup_completion link: true tap = described_class.new("Homebrew", "bar") @@ -320,7 +320,7 @@ describe Tap do specify "#link_completions_and_manpages when completions are enabled for non-official tap" do setup_tap_files setup_git_repo - setup_completion link: "yes" + setup_completion link: true tap = described_class.new("NotHomebrew", "baz") tap.install clone_target: subject.path/".git" (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete @@ -341,7 +341,7 @@ describe Tap do specify "#link_completions_and_manpages when completions are disabled for non-official tap" do setup_tap_files setup_git_repo - setup_completion link: "no" + setup_completion link: false tap = described_class.new("NotHomebrew", "baz") tap.install clone_target: subject.path/".git" (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete @@ -359,7 +359,7 @@ describe Tap do specify "#link_completions_and_manpages when completions are enabled for official tap" do setup_tap_files setup_git_repo - setup_completion link: "no" + setup_completion link: false tap = described_class.new("Homebrew", "baz") tap.install clone_target: subject.path/".git" (HOMEBREW_PREFIX/"share/man/man1/brew-tap-cmd.1").delete diff --git a/Library/Homebrew/utils/analytics.rb b/Library/Homebrew/utils/analytics.rb index 6dca9a2001..9ab040159d 100644 --- a/Library/Homebrew/utils/analytics.rb +++ b/Library/Homebrew/utils/analytics.rb @@ -3,6 +3,7 @@ require "context" require "erb" +require "settings" module Utils # Helper module for fetching and reporting analytics data. @@ -102,27 +103,27 @@ module Utils end def uuid - config_get(:analyticsuuid) + Settings.read :analyticsuuid end def messages_displayed! - config_set(:analyticsmessage, true) - config_set(:caskanalyticsmessage, true) + Settings.write :analyticsmessage, true + Settings.write :caskanalyticsmessage, true end def enable! - config_set(:analyticsdisabled, false) + Settings.write :analyticsdisabled, false messages_displayed! end def disable! - config_set(:analyticsdisabled, true) + Settings.write :analyticsdisabled, true regenerate_uuid! end def regenerate_uuid! # it will be regenerated in next run unless disabled. - config_delete(:analyticsuuid) + Settings.delete :analyticsuuid end def output(args:, filter: nil) @@ -313,25 +314,7 @@ module Utils end def config_true?(key) - config_get(key) == "true" - end - - def config_get(key) - HOMEBREW_REPOSITORY.cd do - Utils.popen_read("git", "config", "--get", "homebrew.#{key}").chomp - end - end - - def config_set(key, value) - HOMEBREW_REPOSITORY.cd do - safe_system "git", "config", "--replace-all", "homebrew.#{key}", value.to_s - end - end - - def config_delete(key) - HOMEBREW_REPOSITORY.cd do - system "git", "config", "--unset-all", "homebrew.#{key}" - end + Settings.read(key) == "true" end def formulae_brew_sh_json(endpoint) From eb3a662841a4ef51aefac13ebcca03df92c283da Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Tue, 12 Jan 2021 16:30:29 -0500 Subject: [PATCH 7/8] completions: clarify that only external tap completions are affected Co-Authored-By: Mike McQuaid --- Library/Homebrew/cmd/completions.rb | 2 +- Library/Homebrew/completions.rb | 2 +- docs/Manpage.md | 2 +- manpages/brew.1 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/cmd/completions.rb b/Library/Homebrew/cmd/completions.rb index 6f0ba352da..3aad3aa9c8 100644 --- a/Library/Homebrew/cmd/completions.rb +++ b/Library/Homebrew/cmd/completions.rb @@ -15,7 +15,7 @@ module Homebrew usage_banner <<~EOS `completions` [] - Control whether Homebrew automatically links shell completion files. + Control whether Homebrew automatically links external tap shell completion files. Read more at . `brew completions` [`state`]: diff --git a/Library/Homebrew/completions.rb b/Library/Homebrew/completions.rb index 356a9dbabd..788949f27f 100644 --- a/Library/Homebrew/completions.rb +++ b/Library/Homebrew/completions.rb @@ -56,7 +56,7 @@ module Completions T.unsafe(self).ohai "Homebrew completions for external commands are unlinked by default!" T.unsafe(self).puts <<~EOS - To opt-in to automatically linking Homebrew shell competion files, run: + To opt-in to automatically linking external tap shell competion files, run: brew completions link Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} EOS diff --git a/docs/Manpage.md b/docs/Manpage.md index 17160e407f..7061af5693 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -96,7 +96,7 @@ Show lists of built-in and external commands. ### `completions` [*`subcommand`*] -Control whether Homebrew automatically links shell completion files. +Control whether Homebrew automatically links external tap shell completion files. Read more at . `brew completions` [`state`] diff --git a/manpages/brew.1 b/manpages/brew.1 index 419c623655..3734944d13 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -94,7 +94,7 @@ List only the names of commands without category headers\. Include aliases of internal commands\. . .SS "\fBcompletions\fR [\fIsubcommand\fR]" -Control whether Homebrew automatically links shell completion files\. Read more at \fIhttps://docs\.brew\.sh/Shell\-Completion\fR\. +Control whether Homebrew automatically links external tap shell completion files\. Read more at \fIhttps://docs\.brew\.sh/Shell\-Completion\fR\. . .P \fBbrew completions\fR [\fBstate\fR] From 4b8477ba70addcc0956ec811e403a67ca711e88b Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Wed, 13 Jan 2021 11:16:09 -0500 Subject: [PATCH 8/8] Completions, Settings: move to Homebrew namespace --- Library/Homebrew/brew.rb | 2 +- Library/Homebrew/completions.rb | 101 +++++++++--------- Library/Homebrew/completions.rbi | 7 ++ Library/Homebrew/settings.rb | 50 ++++----- Library/Homebrew/settings.rbi | 7 ++ Library/Homebrew/tap.rb | 8 +- Library/Homebrew/test/.rubocop_todo.yml | 3 +- Library/Homebrew/test/cmd/completions_spec.rb | 10 +- Library/Homebrew/test/completions_spec.rb | 9 +- Library/Homebrew/test/settings_spec.rb | 2 +- Library/Homebrew/utils/analytics.rb | 14 +-- 11 files changed, 114 insertions(+), 99 deletions(-) create mode 100644 Library/Homebrew/completions.rbi create mode 100644 Library/Homebrew/settings.rbi diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index d234adabc7..a6299cdc44 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -92,7 +92,7 @@ begin internal_cmd ||= begin internal_dev_cmd = Commands.valid_internal_dev_cmd?(cmd) if internal_dev_cmd && !Homebrew::EnvConfig.developer? - Settings.write "devcmdrun", true + Homebrew::Settings.write "devcmdrun", true ENV["HOMEBREW_DEV_CMD_RUN"] = "1" end internal_dev_cmd diff --git a/Library/Homebrew/completions.rb b/Library/Homebrew/completions.rb index 788949f27f..880ac0d8e4 100644 --- a/Library/Homebrew/completions.rb +++ b/Library/Homebrew/completions.rb @@ -4,63 +4,66 @@ require "utils/link" require "settings" -# Helper functions for generating shell completions. -# -# @api private -module Completions - extend T::Sig +module Homebrew + # Helper functions for generating shell completions. + # + # @api private + module Completions + extend T::Sig - module_function + module_function - sig { void } - def link! - Settings.write :linkcompletions, true - Tap.each do |tap| - Utils::Link.link_completions tap.path, "brew completions link" - end - end + SHELLS = %w[bash fish zsh].freeze - sig { void } - def unlink! - Settings.write :linkcompletions, false - Tap.each do |tap| - next if tap.official? - - Utils::Link.unlink_completions tap.path - end - end - - sig { returns(T::Boolean) } - def link_completions? - Settings.read(:linkcompletions) == "true" - end - - sig { returns(T::Boolean) } - def completions_to_link? - shells = %w[bash fish zsh] - Tap.each do |tap| - next if tap.official? - - shells.each do |shell| - return true if (tap.path/"completions/#{shell}").exist? + sig { void } + def link! + Settings.write :linkcompletions, true + Tap.each do |tap| + Utils::Link.link_completions tap.path, "brew completions link" end end - false - end + sig { void } + def unlink! + Settings.write :linkcompletions, false + Tap.each do |tap| + next if tap.official? - sig { void } - def show_completions_message_if_needed - return if Settings.read(:completionsmessageshown) == "true" - return unless completions_to_link? + Utils::Link.unlink_completions tap.path + end + end - T.unsafe(self).ohai "Homebrew completions for external commands are unlinked by default!" - T.unsafe(self).puts <<~EOS - To opt-in to automatically linking external tap shell competion files, run: - brew completions link - Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} - EOS + sig { returns(T::Boolean) } + def link_completions? + Settings.read(:linkcompletions) == "true" + end - Settings.write :completionsmessageshown, true + sig { returns(T::Boolean) } + def completions_to_link? + Tap.each do |tap| + next if tap.official? + + SHELLS.each do |shell| + return true if (tap.path/"completions/#{shell}").exist? + end + end + + false + end + + sig { void } + def show_completions_message_if_needed + return if Settings.read(:completionsmessageshown) == "true" + return unless completions_to_link? + + ohai "Homebrew completions for external commands are unlinked by default!" + puts <<~EOS + To opt-in to automatically linking external tap shell competion files, run: + brew completions link + Then, follow the directions at #{Formatter.url("https://docs.brew.sh/Shell-Completion")} + EOS + + Settings.write :completionsmessageshown, true + end end end diff --git a/Library/Homebrew/completions.rbi b/Library/Homebrew/completions.rbi new file mode 100644 index 0000000000..8c4746bcaf --- /dev/null +++ b/Library/Homebrew/completions.rbi @@ -0,0 +1,7 @@ +# typed: strict + +module Homebrew + module Completions + include Kernel + end +end diff --git a/Library/Homebrew/settings.rb b/Library/Homebrew/settings.rb index 24286026ac..30d5567bd5 100644 --- a/Library/Homebrew/settings.rb +++ b/Library/Homebrew/settings.rb @@ -1,39 +1,41 @@ # typed: true # frozen_string_literal: true -# Helper functions for reading and writing settings. -# -# @api private -module Settings - extend T::Sig +module Homebrew + # Helper functions for reading and writing settings. + # + # @api private + module Settings + extend T::Sig - module_function + module_function - sig { params(setting: T.any(String, Symbol), repo: Pathname).returns(T.nilable(String)) } - def read(setting, repo: HOMEBREW_REPOSITORY) - return unless (repo/".git/config").exist? + sig { params(setting: T.any(String, Symbol), repo: Pathname).returns(T.nilable(String)) } + def read(setting, repo: HOMEBREW_REPOSITORY) + return unless (repo/".git/config").exist? - repo.cd do - Utils.popen_read("git", "config", "--get", "homebrew.#{setting}").chomp.presence + repo.cd do + Utils.popen_read("git", "config", "--get", "homebrew.#{setting}").chomp.presence + end end - end - sig { params(setting: T.any(String, Symbol), value: T.any(String, T::Boolean), repo: Pathname).void } - def write(setting, value, repo: HOMEBREW_REPOSITORY) - return unless (repo/".git/config").exist? + sig { params(setting: T.any(String, Symbol), value: T.any(String, T::Boolean), repo: Pathname).void } + def write(setting, value, repo: HOMEBREW_REPOSITORY) + return unless (repo/".git/config").exist? - repo.cd do - T.unsafe(self).safe_system "git", "config", "--replace-all", "homebrew.#{setting}", value.to_s + repo.cd do + safe_system "git", "config", "--replace-all", "homebrew.#{setting}", value.to_s + end end - end - sig { params(setting: T.any(String, Symbol), repo: Pathname).void } - def delete(setting, repo: HOMEBREW_REPOSITORY) - return unless (repo/".git/config").exist? - return if read(setting, repo: repo).blank? + sig { params(setting: T.any(String, Symbol), repo: Pathname).void } + def delete(setting, repo: HOMEBREW_REPOSITORY) + return unless (repo/".git/config").exist? + return if read(setting, repo: repo).blank? - repo.cd do - T.unsafe(self).safe_system "git", "config", "--unset-all", "homebrew.#{setting}" + repo.cd do + safe_system "git", "config", "--unset-all", "homebrew.#{setting}" + end end end end diff --git a/Library/Homebrew/settings.rbi b/Library/Homebrew/settings.rbi new file mode 100644 index 0000000000..289622fd41 --- /dev/null +++ b/Library/Homebrew/settings.rbi @@ -0,0 +1,7 @@ +# typed: strict + +module Homebrew + module Settings + include Kernel + end +end diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index b9e90b1686..8b3c1ac5fb 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -343,8 +343,8 @@ class Tap command = "brew tap --repair" Utils::Link.link_manpages(path, command) - Completions.show_completions_message_if_needed - if official? || Completions.link_completions? + Homebrew::Completions.show_completions_message_if_needed + if official? || Homebrew::Completions.link_completions? Utils::Link.link_completions(path, command) else Utils::Link.unlink_completions(path) @@ -822,14 +822,14 @@ class TapConfig return unless tap.git? return unless Utils::Git.available? - Settings.read key, repo: tap.path + Homebrew::Settings.read key, repo: tap.path end def []=(key, value) return unless tap.git? return unless Utils::Git.available? - Settings.write key, value.to_s, repo: tap.path + Homebrew::Settings.write key, value.to_s, repo: tap.path end end diff --git a/Library/Homebrew/test/.rubocop_todo.yml b/Library/Homebrew/test/.rubocop_todo.yml index 364b23edc8..7e7fa024f6 100644 --- a/Library/Homebrew/test/.rubocop_todo.yml +++ b/Library/Homebrew/test/.rubocop_todo.yml @@ -22,7 +22,7 @@ RSpec/InstanceVariable: - 'utils/git_spec.rb' - 'version_spec.rb' -# Offense count: 81 +# Offense count: 76 RSpec/MultipleDescribes: Exclude: - 'ENV_spec.rb' @@ -37,7 +37,6 @@ RSpec/MultipleDescribes: - 'cmd/autoremove_spec.rb' - 'cmd/cleanup_spec.rb' - 'cmd/commands_spec.rb' - - 'cmd/completions_spec.rb' - 'cmd/config_spec.rb' - 'cmd/deps_spec.rb' - 'cmd/desc_spec.rb' diff --git a/Library/Homebrew/test/cmd/completions_spec.rb b/Library/Homebrew/test/cmd/completions_spec.rb index 0c093bec6f..721fa184ae 100644 --- a/Library/Homebrew/test/cmd/completions_spec.rb +++ b/Library/Homebrew/test/cmd/completions_spec.rb @@ -3,12 +3,12 @@ require "cmd/shared_examples/args_parse" -describe "Homebrew.completions_args" do - it_behaves_like "parseable arguments" -end +describe "brew completions" do + describe "Homebrew.completions_args" do + it_behaves_like "parseable arguments" + end -describe "brew completions", :integration_test do - it "runs the status subcommand correctly" do + it "runs the status subcommand correctly", :integration_test do HOMEBREW_REPOSITORY.cd do system "git", "init" end diff --git a/Library/Homebrew/test/completions_spec.rb b/Library/Homebrew/test/completions_spec.rb index 41c9f95ef0..efdcee115e 100644 --- a/Library/Homebrew/test/completions_spec.rb +++ b/Library/Homebrew/test/completions_spec.rb @@ -3,7 +3,7 @@ require "completions" -describe Completions do +describe Homebrew::Completions do let(:internal_path) { HOMEBREW_REPOSITORY/"Library/Taps/homebrew/homebrew-bar" } let(:external_path) { HOMEBREW_REPOSITORY/"Library/Taps/foo/homebrew-bar" } @@ -132,12 +132,9 @@ describe Completions do setup_completions external: true delete_completions_setting setting: :completionsmessageshown - # This will fail because the method calls `puts`. - # If we output the `ohai` andcatch the error, we can be usre that the message is showing. - error_message = "private method `puts' called for Completions:Module" + message = /Homebrew completions for external commands are unlinked by default!/ expect { described_class.show_completions_message_if_needed } - .to output.to_stdout - .and raise_error(NoMethodError, error_message) + .to output(message).to_stdout end end end diff --git a/Library/Homebrew/test/settings_spec.rb b/Library/Homebrew/test/settings_spec.rb index 1c42f7fe76..8df4063fc2 100644 --- a/Library/Homebrew/test/settings_spec.rb +++ b/Library/Homebrew/test/settings_spec.rb @@ -3,7 +3,7 @@ require "settings" -describe Settings do +describe Homebrew::Settings do before do HOMEBREW_REPOSITORY.cd do system "git", "init" diff --git a/Library/Homebrew/utils/analytics.rb b/Library/Homebrew/utils/analytics.rb index 9ab040159d..3d20136514 100644 --- a/Library/Homebrew/utils/analytics.rb +++ b/Library/Homebrew/utils/analytics.rb @@ -103,27 +103,27 @@ module Utils end def uuid - Settings.read :analyticsuuid + Homebrew::Settings.read :analyticsuuid end def messages_displayed! - Settings.write :analyticsmessage, true - Settings.write :caskanalyticsmessage, true + Homebrew::Settings.write :analyticsmessage, true + Homebrew::Settings.write :caskanalyticsmessage, true end def enable! - Settings.write :analyticsdisabled, false + Homebrew::Settings.write :analyticsdisabled, false messages_displayed! end def disable! - Settings.write :analyticsdisabled, true + Homebrew::Settings.write :analyticsdisabled, true regenerate_uuid! end def regenerate_uuid! # it will be regenerated in next run unless disabled. - Settings.delete :analyticsuuid + Homebrew::Settings.delete :analyticsuuid end def output(args:, filter: nil) @@ -314,7 +314,7 @@ module Utils end def config_true?(key) - Settings.read(key) == "true" + Homebrew::Settings.read(key) == "true" end def formulae_brew_sh_json(endpoint)