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

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

Merged
merged 1 commit into from
Sep 25, 2022

Conversation

page-down
Copy link
Contributor

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:
#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?
295743a

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

@kovidgoyal
Copy link
Owner

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.

@page-down
Copy link
Contributor Author

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).

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 2, 2022 via email

@page-down
Copy link
Contributor Author

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/python@3.10/bin/python3.10: No module named mypy

57d3d09

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

@kovidgoyal
Copy link
Owner

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

@page-down
Copy link
Contributor Author

... 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.

@kovidgoyal
Copy link
Owner

Link kitty against the python in the venv.

@kovidgoyal
Copy link
Owner

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

@kovidgoyal
Copy link
Owner

This should take care of it: 5ea0519

@kovidgoyal kovidgoyal merged commit 7e4dd8c into kovidgoyal:master Sep 25, 2022
@kovidgoyal
Copy link
Owner

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

@page-down page-down deleted the macos-set-icon branch September 25, 2022 13:24
@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants