-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
3052: clean exit #3075
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
base: master
Are you sure you want to change the base?
3052: clean exit #3075
Conversation
Set TERM variable if it is anything but xterm-256color, this prevents panics and crashes, and logs a warning as well. Updates derailed#3052
This reverts commit a63b480.
I accidentally added a WIP from another local branch to this PR, which I reverted again. I can also scrap and re-create the entire PR / branch if needed. :) |
@derailed anything else I can do so we can merge this MR? Seems to be low hanging fruits IMHO :) |
@KevinGimbel Thanks Kevin! No sure we want to automatically set this. I think it make sense for nix platforms but not sure how it should work on other. Also this is documented and you should only have to do this once if misconfigured?? |
@KevinGimbel Tx Kevin! I think that's a great idea. |
@derailed I completely forgot about this PR 🙈 - I replaced the code so that the variable is not set automatically. Instead a Link to the documentation is printed and k9s exits with status code 1. |
This PR is stale because it has been open for 30 days with no activity. |
A wrong TERM value can cause k9s to crash, instead of crashing we can log a warning and silently set the variable for the duration of k9s' execution with
os.Setenv
. We recommend setting this variable in the README.md anyway (during the Preflight checks)I've tested it with MacOS and would like to test on Linux as well.
How to test
Set TERM to xterm-ghostty
export TERM=xterm-ghostty
Run k9s, it panics:
Run this branch, it works: