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

Replace the deprecated imghdr module #6076

Merged
merged 2 commits into from
Mar 1, 2023
Merged

Conversation

page-down
Copy link
Contributor

When running . /tests.py mypy, it shows the following message and fails (exit code 1).

DeprecationWarning: 'imghdr' is deprecated and slated for removal in Python 3.13

I noticed that the relevant remote control commands cannot be executed via shortcuts.

kitty -o 'map f1 remote_control set-window-logo /path/to/png'
kitty.rc.base.StreamError: No stream_id in rc payload

Since this is related to the remote control protocol, could you update the documentation if you have some time? (And maybe the async part, such as async_id)
https://sw.kovidgoyal.net/kitty/rc_protocol/

The detailed traceback is printed before the Press e to see detailed traceback ... line. This minor issue is not important.

I see the use of NamedTemporaryFile in setting the background image and logo, is it possible to use SHM to avoid higher file system IO (writes) when setting dynamically generated images in high frequency?

Is it intentional not to reload the background image after reloading the configuration?

@kovidgoyal kovidgoyal merged commit cbf3b58 into kovidgoyal:master Mar 1, 2023
@page-down page-down deleted the png branch March 1, 2023 04:58
@kovidgoyal
Copy link
Owner

kovidgoyal commented Mar 1, 2023 via email

@kovidgoyal
Copy link
Owner

kovidgoyal commented Mar 1, 2023 via email

@page-down
Copy link
Contributor Author

... is it possible to use SHM ...

It doesn't actually need a file at all, its just there for code re-use.

In fact, in handle_streamed_data -> StreamInFlight, handle_data is still writing to the temporary file.
The following will not work.
And high frequency updates will result in high file system IO.

cat image.png | kitten @ set-window-logo /dev/stdin
Failed to decode PNG image at: /var/folders/.../.../T/tmpjisxwfhh.png

I noticed the relevant codes.

state.c, set_window_logo
window_logo.c, find_or_create_window_logo
graphics.c, png_path_to_bitmap, `FILE* fp = fopen(path, "r"); ...; log_error("Failed to decode PNG image at: %s", path);`

@kovidgoyal
Copy link
Owner

kovidgoyal commented Mar 1, 2023 via email

@kovidgoyal
Copy link
Owner

Although rather than SHM one can use fmemopen assuming it works on macOS

@page-down
Copy link
Contributor Author

Yes of course, ...

Thanks for the explanation, it's not easy to create a memory file system under macOS.
Unfortunately, even if there are GBs of free memory space, the memory is sitting there and cannot be used.

One use case is to dynamically generate a progress bar or other background task information for the current window.
Rather than generating a temporary file, I would rather send the encoded image directly to the UNIX socket server and render it.

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