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

Rewrite zsh integration #4386

Merged
merged 2 commits into from
Dec 23, 2021
Merged

Rewrite zsh integration #4386

merged 2 commits into from
Dec 23, 2021

Conversation

romkatv
Copy link
Contributor

@romkatv romkatv commented Dec 22, 2021

Note:

  • Very lightly tested.
  • Docs not updated.

Note:

- Very lightly tested.
- Docs not updated.
@romkatv romkatv mentioned this pull request Dec 22, 2021
@romkatv
Copy link
Contributor Author

romkatv commented Dec 22, 2021

The same disclaimer as in #4377 applies here. I personally don't need these changes. If you don't like them, it won't upset me if you close or ignore this PR.

@romkatv
Copy link
Contributor Author

romkatv commented Dec 22, 2021

I've benchmarked the new and the current integration code on Linux x86 and Darwin arm64.

The impact on startup lag (the delay when you start a new shell) went down about by 2x on Linux and by at least 100x on Mac (the latter basically went down to zero). The impact on command lag (the delay every time you press enter) went down by 5x on Linux and 20x on Mac.

I think at the point performance impact is not a serious concern.

@kovidgoyal kovidgoyal merged commit bcecc61 into kovidgoyal:master Dec 23, 2021
@kovidgoyal
Copy link
Owner

I have merged, but I do have one concern. Namely that this leaves us
with a "kitty-integration" function defined. This is annoying because
tab completing kitty now gives kitty-integration as an option. So can we
either hide it which would be ideal (but I dont know if that's possible)
or name it something less likely to conflict when completing.

@romkatv
Copy link
Contributor Author

romkatv commented Dec 23, 2021

Have you tested it? I tested it very little.

By the way, does Kitty have features that depend on extracting the command between OSC 133 B and C? They won't work with zsh because of right prompt and zle redisplay. As far as zsh goes, OSC 133 B and OSC A with k=s aren't useful.

@romkatv
Copy link
Contributor Author

romkatv commented Dec 23, 2021

How about zsh-kitty-integration?

@kovidgoyal
Copy link
Owner

kovidgoyal commented Dec 23, 2021 via email

@romkatv
Copy link
Contributor Author

romkatv commented Dec 23, 2021

We can unfunction it. I'll send you a PR later today.

@romkatv
Copy link
Contributor Author

romkatv commented Dec 23, 2021

We can unfunction it. I'll send you a PR later today.

Sent #4388.

@page-down
Copy link
Contributor

Unrelated to the zsh integration:
I noticed the initial capitalized Kitty, should it be a uniform kitty? I made the change in the new PR.

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.

3 participants