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

Add a startup warning if the TERM var is wrong inside tmux/screen #726

Merged
merged 3 commits into from Jan 8, 2018

Conversation

Projects
None yet
2 participants
@dequis
Copy link
Member

commented Jun 24, 2017

One of the most common and confusing issues we get in #irssi, this should help identifying and mitigating it.


screnshot

12:34 -!- Irssi: warning The TERM environment variable is set to 'xterm' which can cause display glitches when running under tmux.
12:34 -!- Irssi: Consider changing TERM to 'screen' or 'screen-256color' instead.

Feel free to bikeshed the text of the message, not sure i'm entirely happy with it myself.

  • The "can cause display glitches" has a bit too much certainty given how little this code knows about screen-compatible TERM values. Maybe it's ok.

  • I like the tradeoff of suggesting only 'screen' or 'screen-256color' and not mentioning 'tmux' and 'tmux-256color' - while the latter is slightly better, I found it's a bit annoying to set globally and then get errors when you ssh to servers with older terminfo that don't know about it.

  • Mentioning the name of the multiplexer hopefully helps mitigate the unavoidable "it tells me to use screen but i'm using tmux" but it's not foolproof. Not that anything is.

  • I think half of the issues are about someone setting TERM inside .bashrc or equivalent, but some of them have the wrong TERM inside the multiplexer config itself. Could consider guiding users to look in some of those places. Or not.

Out of scope concern: Starting my main irssi connects me to a trillion servers and this message would be lost up there, but the same thing applies to script load failures and other early-startup errors.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2017

yea, weechat has the same warning, wonder if it helps?

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

I'm indifferent but would suggest the help should mention the tmux-terms when running under tmux as well

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2017

@dequis what do to

@dequis

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2017

Oh hey totally forgot I wrote this. Saw the notification and thought "cool, someone wrote it for me".

Re: mentioning TERM=tmux

I like the tradeoff of suggesting only 'screen' or 'screen-256color' and not mentioning 'tmux' and 'tmux-256color' - while the latter is slightly better, I found it's a bit annoying to set globally and then get errors when you ssh to servers with older terminfo that don't know about it.

I think it only exists since ncurses 6.0

So if you're asking for my opinion, I think my opinion is right :P

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2017

the "problem" is that italics only works in tmux if tmux sees that TERM is set to tmux, which is why we should preferably (also) recommend using tmux-* as a term for tmux (however you are right it is not universally available on all shells/systems yet)

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jan 6, 2018

@dequis are you still interested in finishing this up?

@dequis

This comment has been minimized.

Copy link
Member Author

commented Jan 6, 2018

Define "finishing this up"?

I still prefer to mention something that works every time instead of adding more failure modes just to have a minor feature work.

Add a startup warning if the TERM var is wrong inside tmux/screen
One of the most common and confusing issues we get in #irssi,
this should help identifying and mitigating it.

@dequis dequis force-pushed the dequis:term-environment-check branch from 8b6e046 to 0aafd01 Jan 6, 2018

@dequis

This comment has been minimized.

Copy link
Member Author

commented Jan 6, 2018

Rebased and fixed the conflict, feel free to just push stuff if you want to tweak the message.

Reword warning message
Include multiplexer name in TERM recommendation
@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2018

I changed the message because once this lands hopefully ncurses6 is already widespread

@ailin-nemui ailin-nemui added auto-merge and removed needs changes labels Jan 7, 2018

@dequis

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2018

Neat, positional formatting, don't see that often, clever. This is okay.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2018

I actualy screwed up with the space lol

@ailin-nemui ailin-nemui merged commit 2e0815b into irssi:master Jan 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.