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

Starting a Kitty terminal from an ITerm2 terminal causes mdcat to detect iterm2 instead of kitty #230

Closed
norman-abramovitz opened this issue Jan 5, 2023 · 4 comments · Fixed by #232
Assignees
Labels
bug Something isn't working

Comments

@norman-abramovitz
Copy link
Contributor

Starting a kitty terminal from an iterm2 terminal causes mdcat not to detect the kitty correctly.

From the Iterm2 terminal session

$ mdcat --version
mdcat 0.28.0
$ mdcat --detect-only
Terminal: iTerm2
$ echo TERM=$TERM TERM_PROGRAM=$TERM_PROGRAM
TERM=xterm-256color TERM_PROGRAM=iTerm.app
$ kitty &

From the kitty terminal session

$ mdcat --detect-only
Terminal: iTerm2
$ echo TERM=$TERM TERM_PROGRAM=$TERM_PROGRAM
TERM=xterm-kitty TERM_PROGRAM=iTerm.app

Unsetting the environment variable $TERM_PROGRAM makes the mdcat detect the kitty terminal correctly.

$ unset TERM_PROGRAM
$ mdcat --detect-only
Terminal: Kitty

@norman-abramovitz norman-abramovitz changed the title Starting Kitty Terminal from an ITerm2 terminal causes mdcat to detect iterm2 instead of kitty Starting a Kitty terminal from an ITerm2 terminal causes mdcat to detect iterm2 instead of kitty Jan 5, 2023
@swsnr
Copy link
Owner

swsnr commented Jan 5, 2023

Starting a kitty terminal from an iterm2 terminal causes mdcat not to detect the kitty correctly.

Well, this leaks environment variables from iterm2 down to kitty; no wonder this spoils terminal detection. In a way that's even documented; see the manpage which documents which environment variables mdcat looks at and in which order.

I don't think we can fix this; if we changed the order of environment variables we just break different combinations of this 🤷🏿

If you've got an idea how to fix this solid, I welcome a pull request but meanwhile I tend to close this as won't fix, because there's only so much mdcat can do if environment variables of different terminals get mixed up.

@swsnr swsnr added bug Something isn't working help wanted Extra attention is needed labels Jan 5, 2023
@norman-abramovitz
Copy link
Contributor Author

Maybe report a warning since there is an inconsistency between TERM and TERM_PROGRAM?

Since TERM has the word kitty in it, the variable TERM_PROGRAM should be ignored. My understanding of TERM_PROGRAM is that it is additional information, so if TERM has sufficient information, then we do not need to look at TERM_PROGRAM.

https://sw.kovidgoyal.net/kitty/conf/#opt-kitty.term

Nothing solid, but I will look around to see if there are better ways.

@swsnr
Copy link
Owner

swsnr commented Jan 7, 2023

I think we could always check TERM first, because every terminal emulator is probably supposed to set it explicitly, meaning it's much less likely to leak.

I'll refactor terminal detection to make this easier to implement and hopefully solve this issue.

@swsnr swsnr removed the help wanted Extra attention is needed label Jan 7, 2023
@swsnr swsnr self-assigned this Jan 7, 2023
@swsnr
Copy link
Owner

swsnr commented Jan 7, 2023

@norman-abramovitz You may want to test #232; it should fix this issue.

swsnr added a commit that referenced this issue Jan 7, 2023
Introduce a separate TerminalApp struct to abstract terminal detection
separately from terminal capabilities.

Check $TERM before all other environment variables; terminal emulators
always set this variable and if it's something other than
xterm-256colors it's definitely accurate.

Closes GH-230
@swsnr swsnr closed this as completed in #232 Jan 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants