From a062d182c38cf3c0dc850c151b74a90dc7179880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Summer=20=E2=98=80=EF=B8=8F?= Date: Tue, 30 May 2023 20:24:30 -0600 Subject: [PATCH] Support `VISUAL` env var, and prefer it over `EDITOR` --- railties/CHANGELOG.md | 5 +++ railties/lib/rails/command/helpers/editor.rb | 12 ++++--- railties/lib/rails/commands/credentials/USAGE | 4 +-- .../credentials/credentials_command.rb | 2 +- railties/lib/rails/commands/encrypted/USAGE | 2 +- .../commands/encrypted/encrypted_command.rb | 2 +- railties/lib/rails/commands/secrets/USAGE | 4 +-- .../rails/commands/secrets/secrets_command.rb | 2 +- railties/test/commands/credentials_test.rb | 34 +++++++++++++------ railties/test/commands/encrypted_test.rb | 30 +++++++++++----- railties/test/commands/secrets_test.rb | 20 ++++++++--- tasks/release.rb | 5 +-- 12 files changed, 85 insertions(+), 37 deletions(-) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index bb2b78f17d..99a3902d84 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Support `VISUAL` environment variable for commands which open an editor, + and prefer it over `EDITOR`. + + *Summer ☀️* + * Add engine's `test/fixtures` path to `fixture_paths` in `on_load` hook if path exists and is under the Rails application root. diff --git a/railties/lib/rails/command/helpers/editor.rb b/railties/lib/rails/command/helpers/editor.rb index 107e29ff33..d39fe0041f 100644 --- a/railties/lib/rails/command/helpers/editor.rb +++ b/railties/lib/rails/command/helpers/editor.rb @@ -8,11 +8,15 @@ module Command module Helpers module Editor private + def editor + ENV["VISUAL"].to_s.empty? ? ENV["EDITOR"] : ENV["VISUAL"] + end + def display_hint_if_system_editor_not_specified - if ENV["EDITOR"].to_s.empty? - say "No $EDITOR to open file in. Assign one like this:" + if editor.to_s.empty? + say "No $VISUAL or $EDITOR to open file in. Assign one like this:" say "" - say %(EDITOR="mate --wait" #{executable(current_subcommand)}) + say %(VISUAL="mate --wait" #{executable(current_subcommand)}) say "" say "For editors that fork and exit immediately, it's important to pass a wait flag;" say "otherwise, the file will be saved immediately with no chance to edit." @@ -22,7 +26,7 @@ def display_hint_if_system_editor_not_specified end def system_editor(file_path) - system(*Shellwords.split(ENV["EDITOR"]), file_path.to_s) + system(*Shellwords.split(editor), file_path.to_s) end def using_system_editor diff --git a/railties/lib/rails/commands/credentials/USAGE b/railties/lib/rails/commands/credentials/USAGE index 1189bb3a09..8eae046cbc 100644 --- a/railties/lib/rails/commands/credentials/USAGE +++ b/railties/lib/rails/commands/credentials/USAGE @@ -48,8 +48,8 @@ Set up Git to Diff Credentials: To disenroll from this feature, run `<%= executable(:diff) %> --disenroll`. Editing Credentials: - This will open a temporary file in `$EDITOR` with the decrypted contents to edit - the encrypted credentials. + This will open a temporary file in `$VISUAL` or `$EDITOR` with the decrypted + contents to edit the encrypted credentials. When the temporary file is next saved the contents are encrypted and written to `config/credentials.yml.enc` while the file itself is destroyed to prevent credentials diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index f6fde0b2fd..c1308cecf9 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -14,7 +14,7 @@ class CredentialsCommand < Rails::Command::Base # :nodoc: require_relative "credentials_command/diffing" include Diffing - desc "edit", "Open the decrypted credentials in `$EDITOR` for editing" + desc "edit", "Open the decrypted credentials in `$VISUAL` or `$EDITOR` for editing" def edit load_environment_config! load_generators diff --git a/railties/lib/rails/commands/encrypted/USAGE b/railties/lib/rails/commands/encrypted/USAGE index 72c3038346..ed1bf324bf 100644 --- a/railties/lib/rails/commands/encrypted/USAGE +++ b/railties/lib/rails/commands/encrypted/USAGE @@ -16,7 +16,7 @@ Examples: <%= executable(:edit) %> config/encrypted_file.yml.enc - This opens a temporary file in `$EDITOR` with the decrypted contents for editing. + This opens a temporary file in `$VISUAL` or `$EDITOR` with the decrypted contents for editing. To print the decrypted contents of an encrypted file use: diff --git a/railties/lib/rails/commands/encrypted/encrypted_command.rb b/railties/lib/rails/commands/encrypted/encrypted_command.rb index 4d9ba50994..c7d33d5416 100644 --- a/railties/lib/rails/commands/encrypted/encrypted_command.rb +++ b/railties/lib/rails/commands/encrypted/encrypted_command.rb @@ -12,7 +12,7 @@ class EncryptedCommand < Rails::Command::Base # :nodoc: class_option :key, aliases: "-k", type: :string, default: "config/master.key", desc: "The Rails.root relative path to the encryption key" - desc "edit", "Open the decrypted file in `$EDITOR` for editing" + desc "edit", "Open the decrypted file in `$VISUAL` or `$EDITOR` for editing" def edit(*) load_environment_config! diff --git a/railties/lib/rails/commands/secrets/USAGE b/railties/lib/rails/commands/secrets/USAGE index d1f3fca2fd..40eaf2d655 100644 --- a/railties/lib/rails/commands/secrets/USAGE +++ b/railties/lib/rails/commands/secrets/USAGE @@ -53,8 +53,8 @@ Setup: Editing Secrets: After `<%= executable(:setup) %>`, run `<%= executable(:edit) %>`. - That command opens a temporary file in `$EDITOR` with the decrypted contents of - `config/secrets.yml.enc` to edit the encrypted secrets. + That command opens a temporary file in `$VISUAL` or `$EDITOR` with the decrypted + contents of `config/secrets.yml.enc` to edit the encrypted secrets. When the temporary file is next saved the contents are encrypted and written to `config/secrets.yml.enc` while the file itself is destroyed to prevent secrets diff --git a/railties/lib/rails/commands/secrets/secrets_command.rb b/railties/lib/rails/commands/secrets/secrets_command.rb index cbc0ce53dc..307cd8a737 100644 --- a/railties/lib/rails/commands/secrets/secrets_command.rb +++ b/railties/lib/rails/commands/secrets/secrets_command.rb @@ -9,7 +9,7 @@ module Command class SecretsCommand < Rails::Command::Base # :nodoc: include Helpers::Editor - desc "edit", "**deprecated** Open the secrets in `$EDITOR` for editing" + desc "edit", "**deprecated** Open the secrets in `$VISUAL` or `$EDITOR` for editing" def edit Rails.deprecator.warn(<<~MSG.squish) `bin/rails secrets:edit` is deprecated in favor of credentials and will be removed in Rails 7.2. diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index b116ddd07d..4b29ed656f 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -11,13 +11,25 @@ class Rails::Command::CredentialsTest < ActiveSupport::TestCase setup :build_app teardown :teardown_app - test "edit without editor gives hint" do - run_edit_command(editor: "").tap do |output| - assert_match "No $EDITOR to open file in", output + test "edit without visual or editor gives hint" do + run_edit_command(visual: "", editor: "").tap do |output| + assert_match "No $VISUAL or $EDITOR to open file in", output assert_match "rails credentials:edit", output end end + test "edit with visual but not editor does not give hint" do + run_edit_command(visual: "cat", editor: "").tap do |output| + assert_no_match "No $VISUAL or $EDITOR to open file in", output + end + end + + test "edit with editor but not visual does not give hint" do + run_edit_command(visual: "", editor: "cat").tap do |output| + assert_no_match "No $VISUAL or $EDITOR to open file in", output + end + end + test "edit credentials" do # Run twice to ensure credentials can be reread after first edit pass. 2.times do @@ -145,7 +157,7 @@ class Rails::Command::CredentialsTest < ActiveSupport::TestCase assert_match %r/file encrypted and saved/i, run_edit_command interrupt_command_process = %(ruby -e "Process.kill 'INT', Process.ppid") - output = run_edit_command(editor: interrupt_command_process) + output = run_edit_command(visual: interrupt_command_process) assert_no_match %r/file encrypted and saved/i, output assert_match %r/nothing saved/i, output @@ -154,7 +166,7 @@ class Rails::Command::CredentialsTest < ActiveSupport::TestCase test "edit command preserves user's content even if it contains invalid YAML" do write_invalid_yaml = %(ruby -e "File.write ARGV[0], 'foo: bar: bad'") - assert_match %r/WARNING: Invalid YAML/, run_edit_command(editor: write_invalid_yaml) + assert_match %r/WARNING: Invalid YAML/, run_edit_command(visual: write_invalid_yaml) assert_match %r/foo: bar: bad/, run_edit_command end @@ -327,10 +339,12 @@ class Rails::Command::CredentialsTest < ActiveSupport::TestCase private DEFAULT_CREDENTIALS_PATTERN = /access_key_id: 123\n.*secret_key_base: \h{128}\n/m - def run_edit_command(editor: "cat", environment: nil, **options) - switch_env("EDITOR", editor) do - args = environment ? ["--environment", environment] : [] - rails "credentials:edit", args, **options + def run_edit_command(visual: "cat", editor: "cat", environment: nil, **options) + switch_env("VISUAL", visual) do + switch_env("EDITOR", editor) do + args = environment ? ["--environment", environment] : [] + rails "credentials:edit", args, **options + end end end @@ -346,7 +360,7 @@ def run_diff_command(path = nil, enroll: nil, disenroll: nil, **options) def write_credentials(content, **options) switch_env("CONTENT", content) do - run_edit_command(editor: %(ruby -e "File.write ARGV[0], ENV['CONTENT']"), **options) + run_edit_command(visual: %(ruby -e "File.write ARGV[0], ENV['CONTENT']"), **options) end end diff --git a/railties/test/commands/encrypted_test.rb b/railties/test/commands/encrypted_test.rb index e5a7caf39c..a5a793c56e 100644 --- a/railties/test/commands/encrypted_test.rb +++ b/railties/test/commands/encrypted_test.rb @@ -14,13 +14,25 @@ class Rails::Command::EncryptedTest < ActiveSupport::TestCase @encrypted_file = "config/tokens.yml.enc" end - test "edit without editor gives hint" do - run_edit_command(editor: "").tap do |output| - assert_match "No $EDITOR to open file in", output + test "edit without visual or editor gives hint" do + run_edit_command(visual: "", editor: "").tap do |output| + assert_match "No $VISUAL or $EDITOR to open file in", output assert_match "rails encrypted:edit", output end end + test "edit with visual but not editor does not give hint" do + run_edit_command(visual: "cat", editor: "").tap do |output| + assert_no_match "No $VISUAL or $EDITOR to open file in", output + end + end + + test "edit with editor but not visual does not give hint" do + run_edit_command(visual: "", editor: "cat").tap do |output| + assert_no_match "No $VISUAL or $EDITOR to open file in", output + end + end + test "edit encrypted file" do # Run twice to ensure file can be reread after first edit pass. 2.times do @@ -85,7 +97,7 @@ class Rails::Command::EncryptedTest < ActiveSupport::TestCase assert_match %r/file encrypted and saved/i, run_edit_command interrupt_command_process = %(ruby -e "Process.kill 'INT', Process.ppid") - output = run_edit_command(editor: interrupt_command_process) + output = run_edit_command(visual: interrupt_command_process) assert_no_match %r/file encrypted and saved/i, output assert_match %r/nothing saved/i, output @@ -94,7 +106,7 @@ class Rails::Command::EncryptedTest < ActiveSupport::TestCase test "edit command preserves user's content even if it contains invalid YAML" do write_invalid_yaml = %(ruby -e "File.write ARGV[0], 'foo: bar: bad'") - assert_match %r/WARNING: Invalid YAML/, run_edit_command(editor: write_invalid_yaml) + assert_match %r/WARNING: Invalid YAML/, run_edit_command(visual: write_invalid_yaml) assert_match %r/foo: bar: bad/, run_edit_command end @@ -139,9 +151,11 @@ class Rails::Command::EncryptedTest < ActiveSupport::TestCase end private - def run_edit_command(file = @encrypted_file, key: nil, editor: "cat", **options) - switch_env("EDITOR", editor) do - rails "encrypted:edit", prepare_args(file, key), **options + def run_edit_command(file = @encrypted_file, key: nil, visual: "cat", editor: "cat", **options) + switch_env("VISUAL", visual) do + switch_env("EDITOR", editor) do + rails "encrypted:edit", prepare_args(file, key), **options + end end end diff --git a/railties/test/commands/secrets_test.rb b/railties/test/commands/secrets_test.rb index fb0fe8fb4e..2558d58e2b 100644 --- a/railties/test/commands/secrets_test.rb +++ b/railties/test/commands/secrets_test.rb @@ -10,8 +10,16 @@ class Rails::Command::SecretsTest < ActiveSupport::TestCase setup :build_app teardown :teardown_app - test "edit without editor gives hint" do - assert_match "No $EDITOR to open file in", run_edit_command(editor: "") + test "edit without visual or editor gives hint" do + assert_match "No $VISUAL or $EDITOR to open file in", run_edit_command(visual: "", editor: "") + end + + test "edit with visual but not editor does not give hint" do + assert_no_match "No $VISUAL or $EDITOR to open file in", run_edit_command(visual: "cat", editor: "") + end + + test "edit with editor but not visual does not give hint" do + assert_no_match "No $VISUAL or $EDITOR to open file in", run_edit_command(visual: "", editor: "cat") end test "edit secrets" do @@ -44,9 +52,11 @@ def prevent_deprecation end end - def run_edit_command(editor: "cat") - switch_env("EDITOR", editor) do - rails "secrets:edit", allow_failure: true + def run_edit_command(visual: "cat", editor: "cat") + switch_env("VISUAL", visual) do + switch_env("EDITOR", editor) do + rails "secrets:edit", allow_failure: true + end end end diff --git a/tasks/release.rb b/tasks/release.rb index ae5f991a5c..c57947b3ab 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -251,8 +251,9 @@ # Permit the avatar param. substitute.call("app/controllers/users_controller.rb", /:admin/, ":admin, :avatar") - if ENV["EDITOR"] - `#{ENV["EDITOR"]} #{File.expand_path(app_name)}` + editor = ENV["VISUAL"] || ENV["EDITOR"] + if editor + `#{editor} #{File.expand_path(app_name)}` end puts "Booting a Rails server. Verify the release by:"