Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keyboard_layout stanza. #15061

Merged
merged 3 commits into from Mar 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Library/Homebrew/cask/artifact.rb
Expand Up @@ -11,6 +11,7 @@
require "cask/artifact/input_method"
require "cask/artifact/installer"
require "cask/artifact/internet_plugin"
require "cask/artifact/keyboard_layout"
require "cask/artifact/manpage"
require "cask/artifact/vst_plugin"
require "cask/artifact/vst3_plugin"
Expand Down
36 changes: 36 additions & 0 deletions Library/Homebrew/cask/artifact/keyboard_layout.rb
@@ -0,0 +1,36 @@
# typed: true
# frozen_string_literal: true

require "cask/artifact/moved"

module Cask
module Artifact
# Artifact corresponding to the `keyboard_layout` stanza.
#
# @api private
class KeyboardLayout < Moved
extend T::Sig

sig { returns(String) }
def self.english_name
"Keyboard Layout"
end

def install_phase(**options)
super(**options)
delete_keyboard_layout_cache(**options)
end

def uninstall_phase(**options)
super(**options)
delete_keyboard_layout_cache(**options)
end
Comment on lines +19 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is is always necessary to delete the layout cache on both install and uninstall? The PR linked to in the homebrew/cask repo only shows that this is done for one cask.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, https://www.kaufmann.no/roland/dvorak/macosx.html shows its removal in both installation and removal instructions.


private

def delete_keyboard_layout_cache(command: nil, **_)
command.run!("/bin/rm", args: ["-f", "--", "/System/Library/Caches/com.apple.IntlDataCache.le*"], sudo: true)
end
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to take a command argument if it's always going to run the same command?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command argument here contains more information than calling system_command! directly, e.g. whether it should be verbose, so that all commands for a given cask behave the same way in terms of output.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I looked at the other artifacts and see that this is a common pattern. I wonder why it accepts nil as an argument though since it seems to be required here and I couldn't find any references where the command would be nil in the cask installer.

end
end
end
2 changes: 2 additions & 0 deletions Library/Homebrew/cask/config.rb
Expand Up @@ -15,6 +15,7 @@ class Config

DEFAULT_DIRS = {
appdir: "/Applications",
keyboard_layoutdir: "/Library/Keyboard Layouts",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
keyboard_layoutdir: "/Library/Keyboard Layouts",
keyboard_layoutdir: "~/Library/Keyboard Layouts",

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional. You will need sudo to delete the cache anyways, so might as well install in the system-wide directory. Arguably, all of these should use the system-wide directory by default, but it would be pretty hard to migrate these now.

colorpickerdir: "~/Library/ColorPickers",
prefpanedir: "~/Library/PreferencePanes",
qlplugindir: "~/Library/QuickLook",
Expand All @@ -40,6 +41,7 @@ def self.defaults
def self.from_args(args)
new(explicit: {
appdir: args.appdir,
keyboard_layoutdir: args.keyboard_layoutdir,
colorpickerdir: args.colorpickerdir,
prefpanedir: args.prefpanedir,
qlplugindir: args.qlplugindir,
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cask/dsl.rb
Expand Up @@ -44,6 +44,7 @@ class DSL
Artifact::Font,
Artifact::InputMethod,
Artifact::InternetPlugin,
Artifact::KeyboardLayout,
Artifact::Manpage,
Artifact::Pkg,
Artifact::Prefpane,
Expand Down
3 changes: 3 additions & 0 deletions Library/Homebrew/cli/args.rbi
Expand Up @@ -273,6 +273,9 @@ module Homebrew
sig { returns(T.nilable(String)) }
def appdir; end

sig { returns(T.nilable(String)) }
def keyboard_layoutdir; end

sig { returns(T.nilable(String)) }
def fontdir; end

Expand Down
4 changes: 4 additions & 0 deletions Library/Homebrew/cli/parser.rb
Expand Up @@ -37,6 +37,10 @@ def self.global_cask_options
description: "Target location for Applications " \
"(default: `#{Cask::Config::DEFAULT_DIRS[:appdir]}`).",
}],
[:flag, "--keyboard-layoutdir=", {
description: "Target location for Keyboard Layouts " \
"(default: `#{Cask::Config::DEFAULT_DIRS[:keyboard_layoutdir]}`).",
}],
[:flag, "--colorpickerdir=", {
description: "Target location for Color Pickers " \
"(default: `#{Cask::Config::DEFAULT_DIRS[:colorpickerdir]}`).",
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/rubocops/cask/constants/stanza.rb
Expand Up @@ -29,6 +29,7 @@ module Constants
:font,
:input_method,
:internet_plugin,
:keyboard_layout,
:prefpane,
:qlplugin,
:mdimporter,
Expand Down
6 changes: 6 additions & 0 deletions Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi
Expand Up @@ -3007,6 +3007,10 @@ class Cask::Config

def internet_plugindir=(path); end

def keyboard_layoutdir(); end

def keyboard_layoutdir=(path); end

def mdimporterdir(); end

def mdimporterdir=(path); end
Expand Down Expand Up @@ -6554,6 +6558,8 @@ class RuboCop::Cask::AST::Stanza

def internet_plugin?(); end

def keyboard_layout?(); end

def language?(); end

def livecheck?(); end
Expand Down
Expand Up @@ -11,6 +11,7 @@ module Cask
class Config
DEFAULT_DIRS_PATHNAMES = {
appdir: Pathname(TEST_TMPDIR)/"cask-appdir",
keyboard_layoutdir: Pathname(TEST_TMPDIR)/"cask-keyboard-layoutdir",
prefpanedir: Pathname(TEST_TMPDIR)/"cask-prefpanedir",
qlplugindir: Pathname(TEST_TMPDIR)/"cask-qlplugindir",
mdimporterdir: Pathname(TEST_TMPDIR)/"cask-mdimporter",
Expand Down
7 changes: 6 additions & 1 deletion Library/Homebrew/unversioned_cask_checker.rb
Expand Up @@ -31,6 +31,11 @@ def apps
@apps ||= @cask.artifacts.select { |a| a.is_a?(Cask::Artifact::App) }
end

sig { returns(T::Array[Cask::Artifact::Qlplugin]) }
def keyboard_layouts
@keyboard_layouts ||= @cask.artifacts.select { |a| a.is_a?(Cask::Artifact::KeyboardLayout) }
end

sig { returns(T::Array[Cask::Artifact::Qlplugin]) }
def qlplugins
@qlplugins ||= @cask.artifacts.select { |a| a.is_a?(Cask::Artifact::Qlplugin) }
Expand Down Expand Up @@ -93,7 +98,7 @@ def all_versions

installer.extract_primary_container(to: dir)

info_plist_paths = apps.concat(qlplugins, installers).flat_map do |artifact|
info_plist_paths = apps.concat(keyboard_layouts, qlplugins, installers).flat_map do |artifact|
source = artifact.is_a?(Cask::Artifact::Installer) ? artifact.path : artifact.source.basename
top_level_info_plists(Pathname.glob(dir/"**"/source/"Contents"/"Info.plist")).sort
end
Expand Down