Skip to content

Commit 8ef1aca

Browse files
glankkglankk
committed
Fix coloring
1 parent 4617bc2 commit 8ef1aca

9 files changed

Lines changed: 119 additions & 76 deletions

File tree

‎cli/cmdlineparser.cpp‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@
5656
#include <unordered_set>
5757
#include <utility>
5858

59+
#ifdef _WIN32
60+
#include <fcntl.h>
61+
#include <io.h>
62+
#else
63+
#include <fcntl.h>
64+
#include <unistd.h>
65+
#endif
66+
5967
#ifdef HAVE_RULES
6068
// xml is used for rules
6169
#include "xml.h"
@@ -566,6 +574,20 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
566574
mSettings.clangTidyExecutable = argv[i] + 13;
567575
}
568576

577+
else if (std::strncmp(argv[i], "--color=", 8) == 0) {
578+
const std::string value(argv[i] + 8);
579+
if (value == "auto")
580+
mSettings.color = Settings::ColorSetting::automatic;
581+
else if (value == "always")
582+
mSettings.color = Settings::ColorSetting::always;
583+
else if (value == "never")
584+
mSettings.color = Settings::ColorSetting::never;
585+
else {
586+
mLogger.printError("unknown '--colors' value '" + value + "'.");
587+
return Result::Fail;
588+
}
589+
}
590+
569591
else if (std::strncmp(argv[i], "--config-exclude=",17) ==0) {
570592
mSettings.configExcludePaths.insert(Path::fromNativeSeparators(argv[i] + 17));
571593
}
@@ -1553,9 +1575,40 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
15531575
if (mSettings.templateLocation.empty())
15541576
mSettings.templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}";
15551577
}
1556-
// replace static parts of the templates
1557-
substituteTemplateFormatStatic(mSettings.templateFormat);
1558-
substituteTemplateLocationStatic(mSettings.templateLocation);
1578+
1579+
{
1580+
bool enableColors = false;
1581+
1582+
if (mSettings.color == Settings::ColorSetting::automatic && mSettings.outputFormat == Settings::OutputFormat::text) {
1583+
#ifdef _WIN32
1584+
if (mSettings.outputFile.empty())
1585+
enableColors = _isatty(fileno(stderr));
1586+
else {
1587+
int fd = _open(mSettings.outputFile.c_str(), _O_RDONLY);
1588+
if (fd != -1) {
1589+
enableColors = (_isatty(fd) != 0);
1590+
_close(fd);
1591+
}
1592+
}
1593+
#else
1594+
if (mSettings.outputFile.empty())
1595+
enableColors = isatty(fileno(stderr));
1596+
else {
1597+
int fd = open(mSettings.outputFile.c_str(), O_RDONLY | O_NOCTTY);
1598+
if (fd != -1) {
1599+
enableColors = (isatty(fd) != 0);
1600+
close(fd);
1601+
}
1602+
}
1603+
#endif
1604+
}
1605+
else if (mSettings.color == Settings::ColorSetting::always)
1606+
enableColors = true;
1607+
1608+
// replace static parts of the templates
1609+
substituteTemplateFormatStatic(mSettings.templateFormat, enableColors);
1610+
substituteTemplateLocationStatic(mSettings.templateLocation, enableColors);
1611+
}
15591612

15601613
if (mSettings.force || maxconfigs)
15611614
mSettings.checkAllConfigurations = true;
@@ -1724,6 +1777,10 @@ void CmdLineParser::printHelp() const
17241777
" Cppcheck data. After that the normal Cppcheck analysis is\n"
17251778
" used. You must have the executable in PATH if no path is\n"
17261779
" given.\n"
1780+
" --color=<setting> Configure the use of ansi colors in text output. Possible values:\n"
1781+
" * auto Enable when output is a terminal (default)\n"
1782+
" * always Always enable\n"
1783+
" * never Always disable\n"
17271784
" --config-exclude=<dir>\n"
17281785
" Path (prefix) to be excluded from configuration\n"
17291786
" checking. Preprocessor configurations defined in\n"

‎cli/cppcheckexecutor.cpp‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@
7070
#endif
7171

7272
#ifdef _WIN32
73+
#include <io.h>
7374
#include <windows.h>
75+
#else
76+
#include <unistd.h>
7477
#endif
7578

7679
#if !defined(WIN32) && !defined(__MINGW32__)
@@ -612,10 +615,22 @@ void StdLogger::reportErr(const std::string &errmsg)
612615

613616
void StdLogger::reportOut(const std::string &outmsg, Color c)
614617
{
615-
if (c == Color::Reset)
616-
std::cout << ansiToOEM(outmsg, true) << std::endl;
617-
else
618+
bool enableColors = false;
619+
620+
if (mSettings.color == Settings::ColorSetting::automatic) {
621+
#ifdef _WIN32
622+
enableColors = (_isatty(fileno(stdout)) != 0);
623+
#else
624+
enableColors = (isatty(fileno(stdout)) != 0);
625+
#endif
626+
}
627+
else if (mSettings.color == Settings::ColorSetting::always)
628+
enableColors = true;
629+
630+
if (enableColors && c != Color::Reset)
618631
std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
632+
else
633+
std::cout << ansiToOEM(outmsg, true) << std::endl;
619634
}
620635

621636
// TODO: remove filename parameter?

‎lib/color.cpp‎

Lines changed: 1 addition & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -18,56 +18,12 @@
1818

1919
#include "color.h"
2020

21-
#include <cstdlib>
2221
#include <sstream>
2322
#include <iostream>
2423

25-
#ifndef _WIN32
26-
#include <unistd.h>
27-
#endif
28-
29-
bool gDisableColors = false;
30-
31-
#ifndef _WIN32
32-
static bool isStreamATty(const std::ostream & os)
33-
{
34-
static const bool stdout_tty = isatty(STDOUT_FILENO);
35-
static const bool stderr_tty = isatty(STDERR_FILENO);
36-
if (&os == &std::cout)
37-
return stdout_tty;
38-
if (&os == &std::cerr)
39-
return stderr_tty;
40-
return (stdout_tty && stderr_tty);
41-
}
42-
#endif
43-
44-
static bool isColorEnabled(const std::ostream & os)
45-
{
46-
// See https://bixense.com/clicolors/
47-
static const bool color_forced_off = (nullptr != std::getenv("NO_COLOR"));
48-
if (color_forced_off)
49-
{
50-
return false;
51-
}
52-
static const bool color_forced_on = (nullptr != std::getenv("CLICOLOR_FORCE"));
53-
if (color_forced_on)
54-
{
55-
return true;
56-
}
57-
#ifdef _WIN32
58-
(void)os;
59-
return false;
60-
#else
61-
return isStreamATty(os);
62-
#endif
63-
}
64-
6524
std::ostream& operator<<(std::ostream & os, Color c)
6625
{
67-
if (!gDisableColors && isColorEnabled(os))
68-
return os << "\033[" << static_cast<std::size_t>(c) << "m";
69-
70-
return os;
26+
return os << "\033[" << static_cast<std::size_t>(c) << "m";
7127
}
7228

7329
std::string toString(Color c)

‎lib/color.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,4 @@ CPPCHECKLIB std::ostream& operator<<(std::ostream& os, Color c);
3939

4040
CPPCHECKLIB std::string toString(Color c);
4141

42-
extern CPPCHECKLIB bool gDisableColors; // for testing
43-
4442
#endif

‎lib/errorlogger.cpp‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -594,20 +594,35 @@ static void replace(std::string& source, const std::unordered_map<std::string, s
594594
}
595595
}
596596

597-
static void replaceColors(std::string& source) {
598-
// TODO: colors are not applied when either stdout or stderr is not a TTY because we resolve them before the stream usage
599-
static const std::unordered_map<std::string, std::string> substitutionMap =
600-
{
601-
{"{reset}", ::toString(Color::Reset)},
602-
{"{bold}", ::toString(Color::Bold)},
603-
{"{dim}", ::toString(Color::Dim)},
604-
{"{red}", ::toString(Color::FgRed)},
605-
{"{green}", ::toString(Color::FgGreen)},
606-
{"{blue}", ::toString(Color::FgBlue)},
607-
{"{magenta}", ::toString(Color::FgMagenta)},
608-
{"{default}", ::toString(Color::FgDefault)},
609-
};
610-
replace(source, substitutionMap);
597+
static void replaceColors(std::string& source, bool enableColors) {
598+
if (enableColors) {
599+
static const std::unordered_map<std::string, std::string> substitutionMap =
600+
{
601+
{"{reset}", ::toString(Color::Reset)},
602+
{"{bold}", ::toString(Color::Bold)},
603+
{"{dim}", ::toString(Color::Dim)},
604+
{"{red}", ::toString(Color::FgRed)},
605+
{"{green}", ::toString(Color::FgGreen)},
606+
{"{blue}", ::toString(Color::FgBlue)},
607+
{"{magenta}", ::toString(Color::FgMagenta)},
608+
{"{default}", ::toString(Color::FgDefault)},
609+
};
610+
replace(source, substitutionMap);
611+
}
612+
else {
613+
static const std::unordered_map<std::string, std::string> substitutionMap =
614+
{
615+
{"{reset}", ""},
616+
{"{bold}", ""},
617+
{"{dim}", ""},
618+
{"{red}", ""},
619+
{"{green}", ""},
620+
{"{blue}", ""},
621+
{"{magenta}", ""},
622+
{"{default}", ""},
623+
};
624+
replace(source, substitutionMap);
625+
}
611626
}
612627

613628
std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const
@@ -913,16 +928,16 @@ std::string replaceStr(std::string s, const std::string &from, const std::string
913928
return s;
914929
}
915930

916-
void substituteTemplateFormatStatic(std::string& templateFormat)
931+
void substituteTemplateFormatStatic(std::string& templateFormat, bool enableColors)
917932
{
918933
replaceSpecialChars(templateFormat);
919-
replaceColors(templateFormat);
934+
replaceColors(templateFormat, enableColors);
920935
}
921936

922-
void substituteTemplateLocationStatic(std::string& templateLocation)
937+
void substituteTemplateLocationStatic(std::string& templateLocation, bool enableColors)
923938
{
924939
replaceSpecialChars(templateLocation);
925-
replaceColors(templateLocation);
940+
replaceColors(templateLocation, enableColors);
926941
}
927942

928943
std::string getClassification(const std::string &guideline, ReportType reportType) {

‎lib/errorlogger.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,10 @@ class CPPCHECKLIB ErrorLogger {
288288
std::string replaceStr(std::string s, const std::string &from, const std::string &to);
289289

290290
/** replaces the static parts of the location template **/
291-
CPPCHECKLIB void substituteTemplateFormatStatic(std::string& templateFormat);
291+
CPPCHECKLIB void substituteTemplateFormatStatic(std::string& templateFormat, bool enableColors = false);
292292

293293
/** replaces the static parts of the location template **/
294-
CPPCHECKLIB void substituteTemplateLocationStatic(std::string& templateLocation);
294+
CPPCHECKLIB void substituteTemplateLocationStatic(std::string& templateLocation, bool enableColors = false);
295295

296296
/** Get a classification string from the given guideline and reporttype */
297297
CPPCHECKLIB std::string getClassification(const std::string &guideline, ReportType reportType);

‎lib/settings.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,10 @@ class CPPCHECKLIB WARN_UNUSED Settings {
304304
enum class OutputFormat : std::uint8_t {text, plist, sarif, xml};
305305
OutputFormat outputFormat = OutputFormat::text;
306306

307+
/** @brief ansi colors in output */
308+
enum class ColorSetting : std::uint8_t {automatic, always, never};
309+
ColorSetting color = ColorSetting::automatic;
310+
307311
Platform platform;
308312

309313
/** @brief pid of cppcheck. Intention is that this is set in the main process. */

‎test/main.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ int main(int argc, char *argv[])
3131
#endif
3232

3333
Preprocessor::macroChar = '$'; // While macroChar is char(1) per default outside test suite, we require it to be a human-readable character here.
34-
gDisableColors = true;
3534

3635
options args(argc, argv);
3736

‎test/testcolor.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ class TestColor : public TestFixture {
3030
}
3131

3232
void toString() const {
33-
// TODO: color conversion is dependent on stdout/stderr being a TTY
34-
ASSERT_EQUALS("", ::toString(Color::FgRed));
33+
ASSERT_EQUALS("\033[31m", ::toString(Color::FgRed));
3534
}
3635
};
3736

0 commit comments

Comments
 (0)