Strip color from ninja status

[mathstuf]

No description provided.

[mathstuf]

#198 missed this output codepath for stripping ANSI codes.

[mathstuf]

What to do about the test failure? Set CLICOLOR_FORCE=1 since it now detects that color is not supported and strips ANSI sequences?

[jhasse]

Update the test here: https://github.com/ninja-build/ninja/blob/master/misc/output_test.py#L90

Why would you want to strip the codes from the build status in the first place though?

[mathstuf]

I know which test to update, the question is what is the better solution :) .

CMake runs its compiler information detection (built-in include directories, link directories, etc.) as a build step and pipes the output to a file. When that file has ANSI escapes in it, CMake's logic ends up bailing. ANSI color should only go to a terminal unless CLICOLOR_FORCE is in effect, so this seems like the better solution.

[jhasse]

CMake runs its compiler information detection (built-in include directories, link directories, etc.) as a build step and pipes the output to a file.

Sure, why should there be ANSI escape codes in the status lines though?

When that file has ANSI escapes in it, CMake's logic ends up bailing.

Sounds like a bug to me ;)

[mathstuf]

My NINJA_STATUS has color…

[mathstuf]

Sounds like a bug to me ;)

Yes, but that doesn't make this not a bug on Ninja's side too… CMake still has to expect color if CLICOLOR_FORCE=1, so the parser will need fixed too, but I still think this part is a ninja bug.

[jhasse]

CMake pipes the whole Ninja command? Sounds to me, that it should override NINJA_STATUS to be safe.

My NINJA_STATUS has color…

Strictly speaking, that's not a supported configuration yet.

[mathstuf]

Well I had a PR long ago which worked and then the ANSI stuff landed and I haven't had a chance to fix elision again :/ .

That's probably also a solution, but this still seems like a bug to me anyways.

[jhasse]

I'm still thinking about all those frontend changes. To reduce the maintenance burden I would prefer fancy stuff happening in external frontends, e.g. https://github.com/jhasse/ja.

[mathstuf]

Would the existing color support then move out too? If not, I think this still makes sense.

[jhasse]

The existing color support is for build output, not for the build status.

[alexreinking]

CMake runs its compiler information detection (built-in include directories, link directories, etc.) as a build step and pipes the output to a file.

I was just bitten by this today while trying to customize my NINJA_STATUS. This was very hard to diagnose and this PR would have avoided the issue. Using cmake --build wont' call ja.

See also the related CMake issue: https://gitlab.kitware.com/cmake/cmake/-/issues/19316

[torarnv]

As far as I can tell this PR strips ansi color only if the output doesn't support color/ANSI escape sequences, which make sense.

Removing ANSI color completely from NINJA_STATUS doesn't make sense. For example to make the progress a bit nicer:

export NINJA_STATUS=$(echo -e "\e[1;39m[\e[0;34m%f\e[0;39m/\e[35m%t\e[1;39m]\e[0m ")

[mathstuf]

Ah, right see #1494 for a separate approach (which worked before but fails…haven't gotten back to it).