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

feat: add zellij support #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: add zellij support #61

wants to merge 2 commits into from

Conversation

mrdgo
Copy link

@mrdgo mrdgo commented Mar 25, 2024

Very rudimentary draft to support zellij. I primarily wish to get some feedback on what you think is missing.

Depends on: zellij-org/zellij#2835

Copy link
Owner

@vimpostor vimpostor left a comment

Choose a reason for hiding this comment

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

Hey, thanks for taking a look at this, support for zellij is indeed something that is very much wanted.

I wrote a couple of pointers and generally I have the feeling that this probably doesn't need as many code changes, the framework is pretty much already there to accomplish this with just the configuration (see also :h tpipeline-external).

if !empty($TMUX)
let g:tpipeline_size = str2nr(systemlist("sh -c 'echo \"\"; tmux display-message -p \"#{window_width}\"'")[-1])
elseif !empty($ZELLIJ)
" TODO: verify if this is reasonable.
Copy link
Owner

Choose a reason for hiding this comment

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

That's not the correct size as soon as there is a vertical split in zellij. This can also be done by just setting the TpipelineSize autocommand callback, so no code changes needed here.

Copy link
Author

Choose a reason for hiding this comment

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

I tried without, then the STL would be rendered without markers - meaning that the whole line would be rendered on the left side. Currently, zellij has no API to get the width. We probably have to wait.

Copy link
Author

Choose a reason for hiding this comment

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

And also: I don't fully understand the case you tried to describe. In my setup, the bar always takes the full width of the terminal. When I do a split, my editor's width halves but the bar stays in full width and so can the STL.

Copy link
Owner

@vimpostor vimpostor Mar 27, 2024

Choose a reason for hiding this comment

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

tput cols will evaluate to the split's width, not to the full width. You likely didn't notice a difference, because half the width was still enough width.

Try resizing the terminal so that it's width is barely enough to fit the vim statusline. Then split the terminal, and you should notice that the statusline is truncated to half the terminal's width.

let p = "/tmp/tmux-" . systemlist("id -u")[-1]

if !empty($ZELLIJ)
" e.g. /tmp/zjstatus-$UID/$SESSION_NAME-$PANE_ID-vimbridge
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm I'd probably prefer to just use a generic naming scheme, so that the same code path can be used for tmux and zellij.

Copy link
Author

@mrdgo mrdgo Mar 27, 2024

Choose a reason for hiding this comment

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

Very happy to do so! Any idea?

Copy link
Owner

@vimpostor vimpostor Mar 27, 2024

Choose a reason for hiding this comment

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

Something like /tmp/vim-$UID/$SESSION_ID-bridge.
I am curious what you need $PANE_ID for? There is always at most one vim pane focused, which will be rendering the active statusline, no need to support multiple panes at the same time.

In any case this requires some careful planning to not break the config for existing users too much.

Copy link
Author

Choose a reason for hiding this comment

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

That is a very good point!

@@ -1,4 +1,4 @@
if exists('g:loaded_tpipeline') || (empty($TMUX) && !exists("g:tpipeline_refreshcmd")) || !(has('nvim-0.6') ? len(nvim_list_uis()) : (has('job') && has('patch-8.2.4650'))) || has('gui_running')
if exists('g:loaded_tpipeline') || (empty($ZELLIJ) && empty($TMUX) && !exists("g:tpipeline_refreshcmd")) || !(has('nvim-0.6') ? len(nvim_list_uis()) : (has('job') && has('patch-8.2.4650'))) || has('gui_running')
Copy link
Owner

Choose a reason for hiding this comment

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

This is probably also not needed, if instead g:tpipeline_refreshcmd is set to something suitable for zellij (i.e. perhaps manually trigger an update of the statusline, similar to tmux refresh-client -S).

Copy link
Author

@mrdgo mrdgo Mar 27, 2024

Choose a reason for hiding this comment

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

Yeah, for sure. However that is not possible as of now. We have to wait for pipes in zellij zellij-org/zellij#3066. It is already merged but not released yet. So currently we have no way to manually trigger an update - my config updates the bar every second.

Copy link
Owner

Choose a reason for hiding this comment

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

OK let's wait for zellij's next release then.

Copy link
Owner

Choose a reason for hiding this comment

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

Looks like it's possible to trigger updates from external now: https://zellij.dev/news/welcome-screen-pipes-filepicker/#pipes
E.g.: zpipe my-plugin hi!

So with dj95/zjstatus#46 it should be pretty easy now to get this working with zellij, probably doesn't need many code changes.

Copy link
Author

Choose a reason for hiding this comment

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

true, I will look into it as soon as time allows me to :)

Very rudimentary, draft to support zellij.
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