-
-
Notifications
You must be signed in to change notification settings - Fork 681
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
Display session name within zellij
#608
Display session name within zellij
#608
Conversation
Thank you for this pr!
I like it much more after your change. I think we thought already about moving it.
How did you set up your toolchain? Does the
The And a little bit general feedback: For me personally the string is a bit long. This is all still a little bit complicated since we haven't fleshed out |
This is looking quite cool in general! Just a note about this bit:
The compelling reason here is that everything in I'd personally go back to the |
Checking over things again, it came from setting [build]
rustflags = "-C link-arg=-fuse-ld=lld Building the plugins just had a lot of text about ignoring settings caused by having
I went ahead and changed it to just display the session name (falling back to
I think that's a very good reasoning. I went ahead and moved it back into |
@TheLostLambda thank you for the insight - that makes a lot of sense.
Thank you for changing that, but I think if it is not configurable yet, then the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this awesome pr! With this little change I think it is ready to be merged.
This reverts commit b25842a.
Sorry life got busy (moving and all), so I left this PR out to dry on accident. I went ahead and reverted the commit so it's back to displaying I think I'll leave the current method for getting the session name as well instead of trying to weave that information through everything (although if you all want that changed then feel free to do so! I'm sure it would be much easier for people that are more familiar with the codebase) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, life has priority! I wasn't sure if I was clear enough that's why I tried to explain it in a different way.
Thank you for this awesome work.
I think this looks good for now.
* Missed something while merging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much! I went ahead and merged it now.
@LovecraftianHorror Thank you so much! |
Closes #573
This displays the session name next to
Zellij
in the top-left corner by passing the information inModeInfo
as described in #573. Example:The session name is stored as an
Option<String>
so if, for whatever reason,None
gets passed to the plugins then the name will fall back to the old style of just displayingZellij
. I did this because theState
for the plugin has to implementDefault
, but I haven't been able to see a situation where it receivesNone
.This currently gets the session name by reading the
ZELLIJ_SESSION_NAME
env var, but I would prefer to have the name passed in as aString
instead and am planning on making this change before switching this off a draft.I did also move
get_mode_info()
intozellij_tile
asModeInfo::new()
since it seemed like the function was just serving as a constructor forModeInfo
, but let me know if there was any compelling reason to keep it as a separate function in a different crate.Also just to note from a new contributor I had to:
-C target-cpu=native
from myrustflags
to get the wasm portions to build without erroringrustup
along with the musl C standard library from my package managerrustup target add x86_64-unknown-linux-musl
sudo pacman -S musl
for Arch at least