kitty icon indicating copy to clipboard operation
kitty copied to clipboard

macOS: Add a way to set a custom application icon for kitty

Open page-down opened this issue 3 years ago • 3 comments
trafficstars

Changing the kitty icon manually as described in the FAQ is troublesome, so a new way to set custom icons via the CLI has been added.

Configure custom icon for the app bundle directly by calling the Cocoa framework's API.

Related discussion: https://github.com/kovidgoyal/kitty/issues/5461

This PR does not fix the above issue because it does not yet add to the installer script the steps to: detect the presence of custom icon, backup, restore custom icon, etc, as this would add unnecessary complexity to the installation script.

Please review, thanks.

I noticed the following commit, does that mean that using doc text roles without the slash / prefix is no longer supported? https://github.com/kovidgoyal/kitty/commit/295743a96e21ffe5fadb67e2336effc140e29798

:doc:`launch` -> :doc:`/launch`

page-down avatar Sep 02 '22 12:09 page-down

Why not just use this method at kitty startup to check for kitty.app.icns in the kitty config folder and set the icon. All the user needs to do is place the icon in the config directory. Ideally one should check if the icon needs updating rather than running it every time, of course. This can be done by storing a size+timestamp of the icns file in the cache dir and checking if it has changed or if there is no custom icon in the bundle.

kovidgoyal avatar Sep 02 '22 12:09 kovidgoyal

I have never seen another app use this approach and am concerned about potential unknown issues.

The cache information needs to be attached to the app bundle, as each app bundle can have its own custom icon. Also, if a kitty.app with a dev label on the custom icon is run, it will be overwritten by kitty.app.icns.

I think the easiest way to do this is: Automatically apply the custom icon if it exists in the configuration folder and the current app bundle does not have a custom icon. (However this will add one more file IO at startup.) Update the icons by running rm "/Applications/kitty.app/Icon"$'\r', or delete the custom icons via Finder, or overwrite with an unmodified kitty.app and run kitty again. Or just drag and drop your custom icon in Finder.

Automatic updates: If there are no custom icon, kitty.app.icns in the configuration folder will be used automatically and the path to the icns file (and file size, mtime..) will be recorded via the extended attributes (xattr) on the app bundle folder. Update the custom icon automatically based on the information in xattr.

I will try the former approach and test it to reduce unnecessary code maintenance in the future (as compared to the second approach).

page-down avatar Sep 02 '22 13:09 page-down

On Fri, Sep 02, 2022 at 06:25:46AM -0700, page-down wrote:

I have never seen another app use this approach and am concerned about potential unknown issues.

This will be used by a small fraction of kitty users, and if there are issues, we can always revert it.

The cache information needs to be attached to the app bundle, as each app bundle can have its own custom icon. Also, if a kitty.app with a dev label on the custom icon is run, it will be overwritten by kitty.app.icns.

I think the easiest way to do this is: Automatically apply the custom icon if it exists in the configuration folder and the current app bundle does not have a custom icon. (However this will add one more file IO at startup.) Update the icons by running rm "/Applications/kitty.app/Icon"$'\r', or delete the custom icons via Finder, or overwrite with an unmodified kitty.app and run kitty again. Or just drag and drop your custom icon in Finder.

Automatic updates: If there are no custom icon, kitty.app.icns in the configuration folder will be used automatically and the path to the icns file (and file size, mtime..) will be recorded via the extended attributes (xattr) on the app bundle folder. Update the custom icon automatically based on the information in xattr.

I will try the former approach and test it to reduce unnecessary code maintenance in the future (as compared to the second approach).

OK.

kovidgoyal avatar Sep 02 '22 13:09 kovidgoyal

I have updated the PR, please review, thanks.

Is it possible to run mypy tests in a virtual environment?

python3 -m venv /path/to/venv
# activate venv
make
./test.py mypy
python3 test.py mypy
/usr/local/opt/[email protected]/bin/python3.10: No module named mypy

https://github.com/kovidgoyal/kitty/commit/57d3d0967940d50c9ca9a8a79a6ec847f8d7ad28

After the virtual environment has been activated:
`PATH` -> /path/to/kitty/launcher:/usr/local/bin:/usr/bin:...
`shutil.which('python')` -> /usr/local/bin/python

page-down avatar Sep 25 '22 07:09 page-down

No idea, never tried as I dont use venvs. Presumably you will need to install mypy in the venv.

kovidgoyal avatar Sep 25 '22 07:09 kovidgoyal

... Presumably you will need to install mypy in the venv.

Yes, mypy is installed. Everything was working fine before, however, after the above linked commit, python outside of venv is called.

page-down avatar Sep 25 '22 07:09 page-down

Link kitty against the python in the venv.

kovidgoyal avatar Sep 25 '22 08:09 kovidgoyal

Actually no, the linked interpreter is not used. See the function type_check() in main.py

kovidgoyal avatar Sep 25 '22 09:09 kovidgoyal

This should take care of it: https://github.com/kovidgoyal/kitty/commit/5ea0519f62c8562633f269ce0ea147629ea6334a

kovidgoyal avatar Sep 25 '22 09:09 kovidgoyal

I have merged changing the implementation a bit, to allow the icon to be updated easily.

kovidgoyal avatar Sep 25 '22 12:09 kovidgoyal

On Fri, Sep 02, 2022 at 05:09:20AM -0700, page-down wrote:

I noticed the following commit, does that mean that using doc text roles without the slash / prefix is no longer supported? https://github.com/kovidgoyal/kitty/commit/295743a96e21ffe5fadb67e2336effc140e29798

No it works fine. It does not work in sphinx because there doc roles are relative to the file, not the root of the tree. This has always been the case.

kovidgoyal avatar Oct 11 '22 07:10 kovidgoyal