Ticket #4981: do not show color warning with --nocolor

[peripherium]

Proposed changes

Suppress the color capability warning when --nocolor is used.

The warning is still shown if a skin is explicitly requested via menu.

Additionally, the global variable mc_skin_is_init was removed:
it is set with seemingly meaningful values but never actually used.

• Resolves: --nocolor complains about not enough colors #4981

Checklist

• I have referenced the issue(s) resolved by this PR (if any)
• I have signed-off my contribution with git commit --amend -s
• Lint and unit tests pass locally with my changes (make indent && make check)
• I have added tests that prove my fix is effective or that my feature works
• I have added the necessary documentation (if appropriate)

---

Quick question: in earlier commits I was asked to add a test function
even when the function already existed. Should a test function be
added for every existing function in the future? If so, I can add
one for this change as well.

[zyv]

Quick question: in earlier commits I was asked to add a test function even when the function already existed. Should a test function be added for every existing function in the future? If so, I can add one for this change as well.

In general, I tend to ask for a test when 1) the changes seem "risky" according to my subjective feeling and 2) it seems to me that they can indeed be tested relatively simply with the current setup.

I mostly have a feeling that changes are "risky" when an old large function full of loops and conditionals, and possibly external variables, which is otherwise not covered with tests, is changed, and likely in a non-trivial fashion.

In this case, I wouldn't demand a test, but, of course, every little bit is welcome, if you feel like it. Our test coverage is very bad, and this is one of the reasons why it's so hard to review PRs in a timely manner. You have to check that the code makes sense and doesn't have unwanted side-effects, and with no test coverage it's mostly a gamble...

[peripherium]

In this case, I wouldn't demand a test, but, of course, every little bit is welcome, if you feel like it. Our test coverage is very bad, and this is one of the reasons why it's so hard to review PRs in a timely manner. You have to check that the code makes sense and doesn't have unwanted side-effects, and with no test coverage it's mostly a gamble...

I took a quick look at this over the weekend and decided to add a test. It turned out to be fairly simple, and writing it was a nice way to better understand this part of the code.

[peripherium]

I don't understand why my mc_skin_init() test fails on both Ubuntu and Fedora. Is there a way to look at the log somewhere?

[zyv]

I don't understand why my mc_skin_init() test fails on both Ubuntu and Fedora.

On Fedora, it's a simple formatting issue, and you have accidentally botched a symlink. You can see the log here:

https://github.com/MidnightCommander/mc/actions/runs/23750102108/job/69189411316?pr=5081

Is there a way to look at the log somewhere?

On Ubuntu, it's more interesting. Maybe something to do with mocking?

FAIL: common__mc_skin_init
==========================

Running suite(s): /lib/skin
Running suite /lib/skin
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:0: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:152:P:Core:skin_test:1: Passed
../../../../tests/lib/skin/common__mc_skin_init.c:152:P:Core:skin_test:2: Passed
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:3: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:152:P:Core:skin_test:4: Passed
../../../../tests/lib/skin/common__mc_skin_init.c:152:P:Core:skin_test:5: Passed
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:6: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:7: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:152:P:Core:skin_test:8: Passed
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:9: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:10: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:152:P:Core:skin_test:11: Passed
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:12: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:13: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:14: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:15: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:16: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:17: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
33%: Checks: 18, Failures: 12, Errors: 0
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:0: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:3: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:6: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:7: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:9: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:10: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:12: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:13: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:14: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:15: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:16: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
../../../../tests/lib/skin/common__mc_skin_init.c:145:F:Core:skin_test:17: mc_skin_init (data->skin_name, &mcerror) variable should be TRUE
Results for all suites run:
33%: Checks: 18, Failures: 12, Errors: 0
FAIL common__mc_skin_init (exit status: 1)

[peripherium]

On Ubuntu, it's more interesting. Maybe something to do with mocking?

I've added a line to my test code (printf("MOCK: tty_use_256colors()\n");) that proves, the Ubuntu test ignores the "weak" attribute (MC_MOCKABLE). I'm currently comparing my test with other tests that mock functions. Any suggestions or ideas are welcome.

[zyv]

@peripherium I think that I've now fixed the CI and reverted all of your experimental changes. Please have another look.

The problem was that you failed to account for VPATH builds, and so your hacked skin loading wasn't working correctly. The other irritating problem with library linking order is due to #4806. I hope that one day I'll get enough time to fix this. Sorry that it has taken me so long.

@mc-worker @egmontkob, are you fine with the current state of the PR?

[zyv]

/rebase