-
-
Notifications
You must be signed in to change notification settings - Fork 54
Tickets #4137 & #4821: true color support with ncurses and S-Lang color detection #4822
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
Conversation
2f5ff38 to
cfa0d08
Compare
|
Just checked what I see the bit in your change. I guess this is the best we can do, given the underlying library's limitations. |
egmontkob
left a comment
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.
I'm not going to do a detailed review. As for the big picture, I think it's as good as it gets (given ncurses's limitations), and it survived nicely some stress-testing for me (like using a 256-color skin with direct16, rejected with an error message as expected).
Thanks a lot!
Yes, unfortunately. I’m trying to figure out better solution with @ThomasDickey, but the discussion is still not completed - see: https://lists.gnu.org/archive/html/bug-ncurses/2025-09/msg00032.html (the rest of this thread is there: https://lists.gnu.org/archive/html/bug-ncurses/2025-08/msg00101.html) |
|
I wonder if the build failure on Solaris is again due to the
|
I completely understand :) That’s because ncurses doesn’t recognize init_extended_pair(), which means it’s using a version of ncurses older than 6.1 (from 2018). Maybe #ifdef that? |
2bdb766 to
8de0488
Compare
Done |
b0dbf5a to
7aa592b
Compare
|
It appears that Solaris build still fails because although it uses recent ncurses, but it is configured to use ABI 5, which is incompatible with init_extended_pair(). Included into #ifdef. |
ddb713c to
95a0a13
Compare
|
Fixed compatibility code in 8-color mode, improved documentation, included commit with minor improvement of true color skins (differentiate directories and files) |
95a0a13 to
d2da004
Compare
|
Fixed #ifdefs for checking availability of ncurses extended colors |
|
@horkykuba You rock! (I've verified your widechar detection.) @zyv You overlooked this, there's another commit underneath similar to the one you suggested, combined with my suggestion to use a more verbose name. @zyv Could we please land these two commits in master ASAP?
My line drawing work also depends on that; that way I could go on in parallel to the beefier truecolor review. |
I'm sorry, I'm getting really confused and overwhelmed by the email flow :-/
If you guys could please extract this change as a separate PR, I will merge it immediately. Then you can rebase your other WIP stuff on top of that... |
Done, #4826. |
d3a7ba3 to
9df375c
Compare
|
Added commit with compatibility translation from 256 colors to true colors if the terminal supports true colors but doesn't support 256 colors. |
ossilator
left a comment
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.
i just skimmed it (and don't intend to go further).
So, we have confirmed the new capability names for separate RGB, intended to be included in terminfo and ncurses by @ThomasDickey in the future: https://lists.gnu.org/archive/html/bug-ncurses/2025-10/msg00086.html It follows today's Emacs implementation. |
5d0c98f to
7e95d09
Compare
|
Made changes suggested by @ossilator and improved documentation in comments |
zyv
left a comment
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.
What do you think @mc-worker & @egmontkob ? I'm not a color-user, but code-wise I don't think I have anything to request, so I'm wondering if we can merge it now.
|
I only skimmed over the first iteration the code, on the high level it looked about as reasonable (incl. workarounds for ncurses's limitations) as it could be, also survived a bit of testing from me. Also, I know I shouldn't go personal, but based on his debut contributions I have a pretty good trust in horkykuba's work. Yup let's land it and see how it goes. |
|
Thank you guys. Fyi, I also plan to add support for
However, I'll do that in a separate PR. |
|
/rebase |
Signed-off-by: Jakub Horký <[email protected]>
Display directories in a more prominent color than files, as in most other skins. Signed-off-by: Jakub Horký <[email protected]>
… detection Enable true color support in ncurses. TERM must be set to a relevant terminfo entry, i.e. *-direct, *-direct16, or *-direct256. When set to *-direct256, both 256-color and true color mode can be used at the same time. Also fix true color detection in S-Lang. S-Lang checks for RGB terminfo capability to enable true color, so we should check for it as well. This means the COLORTERM variable doesn't need to be set when using direct terminfo variants. This also applies to simultaneous 256-color and true color mode. We need to access the RGB extended capability. This is supported in both S-Lang and ncurses, so unify the terminfo access routines to enable it. Now they should be called with both terminfo and termcap capability name. Update FAQ. Resolves: MidnightCommander#4137 Resolves: MidnightCommander#4821 Signed-off-by: Jakub Horký <[email protected]>
If the terminal does not support 256 colors but does support true color, allow the use of 256-color skins by adding a translation layer that computes direct color values from 256-color palette. Signed-off-by: Jakub Horký <[email protected]>
7e95d09 to
5633ffe
Compare
|
@horkykuba, thank you for your contribution! |
Proposed changes
Enable true color support in ncurses. TERM must be set to a relevant terminfo entry, i.e. *-direct, *-direct16, or *-direct256.
If the terminal does not support 256 colors but does support true color, allow the use of 256-color skins by adding a translation layer that computes direct color values from 256-color palette. So both 256-color and true color mode can be used at the same time.
Also fix true color detection in S-Lang. S-Lang checks for RGB terminfo capability to enable true color, so we should check for it as well. This means the COLORTERM variable doesn't need to be set when using direct terminfo variants. This also applies to simultaneous 256-color and true color mode.
Update FAQ.
Checklist
👉 Our coding style can be found here: https://midnight-commander.org/coding-style/ 👈
git commit --amend -smake indent && make check)