Skip to content

Commit 5ce8eb4

Browse files
glankkglankk
committed
Fix coloring
1 parent 4617bc2 commit 5ce8eb4

8 files changed

Lines changed: 99 additions & 57 deletions

File tree

‎cli/cmdlineparser.cpp‎

Lines changed: 40 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"
@@ -1553,9 +1561,38 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
15531561
if (mSettings.templateLocation.empty())
15541562
mSettings.templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}";
15551563
}
1556-
// replace static parts of the templates
1557-
substituteTemplateFormatStatic(mSettings.templateFormat);
1558-
substituteTemplateLocationStatic(mSettings.templateLocation);
1564+
1565+
{
1566+
bool enableColors = false;
1567+
1568+
if (!getForcedColorSetting(enableColors) && mSettings.outputFormat == Settings::OutputFormat::text) {
1569+
#ifdef _WIN32
1570+
if (mSettings.outputFile.empty())
1571+
enableColors = _isatty(_fileno(stderr));
1572+
else {
1573+
int fd = _open(mSettings.outputFile.c_str(), _O_RDONLY);
1574+
if (fd != -1) {
1575+
enableColors = (_isatty(fd) != 0);
1576+
_close(fd);
1577+
}
1578+
}
1579+
#else
1580+
if (mSettings.outputFile.empty())
1581+
enableColors = isatty(fileno(stderr));
1582+
else {
1583+
int fd = open(mSettings.outputFile.c_str(), O_RDONLY | O_NOCTTY);
1584+
if (fd != -1) {
1585+
enableColors = (isatty(fd) != 0);
1586+
close(fd);
1587+
}
1588+
}
1589+
#endif
1590+
}
1591+
1592+
// replace static parts of the templates
1593+
substituteTemplateFormatStatic(mSettings.templateFormat, enableColors);
1594+
substituteTemplateLocationStatic(mSettings.templateLocation, enableColors);
1595+
}
15591596

15601597
if (mSettings.force || maxconfigs)
15611598
mSettings.checkAllConfigurations = true;

‎cli/cppcheckexecutor.cpp‎

Lines changed: 16 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,20 @@ 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;
619+
620+
if (!getForcedColorSetting(enableColors)) {
621+
#ifdef _WIN32
622+
enableColors = (_isatty(_fileno(stdout)) != 0);
623+
#else
624+
enableColors = (isatty(fileno(stdout)) != 0);
625+
#endif
626+
}
627+
628+
if (enableColors && c != Color::Reset)
618629
std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
630+
else
631+
std::cout << ansiToOEM(outmsg, true) << std::endl;
619632
}
620633

621634
// TODO: remove filename parameter?

‎lib/color.cpp‎

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,48 +26,27 @@
2626
#include <unistd.h>
2727
#endif
2828

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)
29+
bool getForcedColorSetting(bool &colorSetting)
4530
{
4631
// See https://bixense.com/clicolors/
4732
static const bool color_forced_off = (nullptr != std::getenv("NO_COLOR"));
4833
if (color_forced_off)
4934
{
50-
return false;
35+
colorSetting = false;
36+
return true;
5137
}
5238
static const bool color_forced_on = (nullptr != std::getenv("CLICOLOR_FORCE"));
5339
if (color_forced_on)
5440
{
41+
colorSetting = true;
5542
return true;
5643
}
57-
#ifdef _WIN32
58-
(void)os;
5944
return false;
60-
#else
61-
return isStreamATty(os);
62-
#endif
6345
}
6446

6547
std::ostream& operator<<(std::ostream & os, Color c)
6648
{
67-
if (!gDisableColors && isColorEnabled(os))
68-
return os << "\033[" << static_cast<std::size_t>(c) << "m";
69-
70-
return os;
49+
return os << "\033[" << static_cast<std::size_t>(c) << "m";
7150
}
7251

7352
std::string toString(Color c)

‎lib/color.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#include <ostream>
2626
#include <string>
2727

28+
bool getForcedColorSetting(bool &colorSetting);
29+
2830
enum class Color : std::uint8_t {
2931
Reset = 0,
3032
Bold = 1,
@@ -39,6 +41,4 @@ CPPCHECKLIB std::ostream& operator<<(std::ostream& os, Color c);
3941

4042
CPPCHECKLIB std::string toString(Color c);
4143

42-
extern CPPCHECKLIB bool gDisableColors; // for testing
43-
4444
#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);

‎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)