tty: add color support for mosh#27843
tty: add color support for mosh#27843aditya1906 wants to merge 5 commits intonodejs:masterfrom aditya1906:master
Conversation
BridgeAR
left a comment
There was a problem hiding this comment.
Would you be wo kind and verify process.env.TERM in the Node.js REPL while using mosh and post the output here? AFAIK the value is set to xterm-256?
lib/internal/tty.js
Outdated
| 'konsole': COLORS_16, | ||
| 'kterm': COLORS_16, | ||
| 'mlterm': COLORS_16, | ||
| 'mosh': COLORS_256, |
There was a problem hiding this comment.
It seems like mosh has true color support?
There was a problem hiding this comment.
Hi @BridgeAR Mosh has xterm-256 support. It doesn't have truecolor support. I took a look in mosh repository and found that it has xterm-256 support and not truecolor. So shouldn't it be COLORS_256? Or what is the value for xterm-256? Please let me know if I'm mistaken.
There was a problem hiding this comment.
mosh gained truecolor support a few years ago.
There was a problem hiding this comment.
Hi @devsnek yes I read that it is now supporting truecolor. So is my commit worth merging to the master branch? Or should it be COLORS_16?
There was a problem hiding this comment.
you should change it to COLORS_16m
There was a problem hiding this comment.
I have corrected the mistake
|
Hi I made the corrections for mosh. Is it good enough? |
|
@aditya1906 would you be so kind and make a screenshot of the corresponding environment variable entry? That way it's easier to verify that it's set as expected :) |
|
Hi @BridgeAR |
|
Fwiw, the commit message here should start with |
|
What do you think about this? |
|
@aditya1906 you should change the commit message rather than the PR title. https://help.github.com/en/articles/changing-a-commit-message this will help you learn how to change commit message. |
|
@mbj36 I think in this case it’s fine – we’ll squash the commits anyway when merging. My message was more supposed to inform the person who merges this than @aditya1906 (although of course it’s great if the commit messages here are changed as well). |
PR-URL: #27843 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
Landed in 70abb4f |
PR-URL: #27843 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #27843 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Added mosh to the tty.js file for terminal color support #27609