Skip to content

Commit 6dcf71c

Browse files
glankkglankk
committed
Quick fix
1 parent ed25455 commit 6dcf71c

8 files changed

Lines changed: 75 additions & 100 deletions

File tree

‎cli/cmdlineparser.cpp‎

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,6 @@
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-
6759
#ifdef HAVE_RULES
6860
// xml is used for rules
6961
#include "xml.h"
@@ -1561,38 +1553,9 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
15611553
if (mSettings.templateLocation.empty())
15621554
mSettings.templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}";
15631555
}
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-
}
1556+
// replace static parts of the templates
1557+
substituteTemplateFormatStatic(mSettings.templateFormat, !mSettings.outputFile.empty());
1558+
substituteTemplateLocationStatic(mSettings.templateLocation, !mSettings.outputFile.empty());
15961559

15971560
if (mSettings.force || maxconfigs)
15981561
mSettings.checkAllConfigurations = true;

‎cli/cppcheckexecutor.cpp‎

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

7272
#ifdef _WIN32
73-
#include <io.h>
7473
#include <windows.h>
75-
#else
76-
#include <unistd.h>
7774
#endif
7875

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

616613
void StdLogger::reportOut(const std::string &outmsg, Color c)
617614
{
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)
629-
std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
630-
else
615+
if (c == Color::Reset)
631616
std::cout << ansiToOEM(outmsg, true) << std::endl;
617+
else
618+
std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
632619
}
633620

634621
// TODO: remove filename parameter?

‎lib/color.cpp‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,52 @@
2222
#include <sstream>
2323
#include <iostream>
2424

25-
bool getForcedColorSetting(bool &colorSetting)
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)
2645
{
2746
// See https://bixense.com/clicolors/
2847
static const bool color_forced_off = (nullptr != std::getenv("NO_COLOR"));
2948
if (color_forced_off)
3049
{
31-
colorSetting = false;
32-
return true;
50+
return false;
3351
}
3452
static const bool color_forced_on = (nullptr != std::getenv("CLICOLOR_FORCE"));
3553
if (color_forced_on)
3654
{
37-
colorSetting = true;
3855
return true;
3956
}
57+
#ifdef _WIN32
58+
(void)os;
4059
return false;
60+
#else
61+
return isStreamATty(os);
62+
#endif
4163
}
4264

4365
std::ostream& operator<<(std::ostream & os, Color c)
4466
{
45-
return os << "\033[" << static_cast<std::size_t>(c) << "m";
67+
if (!gDisableColors && isColorEnabled(os))
68+
return os << "\033[" << static_cast<std::size_t>(c) << "m";
69+
70+
return os;
4671
}
4772

4873
std::string toString(Color c)

‎lib/color.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,10 @@ enum class Color : std::uint8_t {
3535
FgMagenta = 35,
3636
FgDefault = 39
3737
};
38-
39-
CPPCHECKLIB bool getForcedColorSetting(bool &colorSetting);
40-
4138
CPPCHECKLIB std::ostream& operator<<(std::ostream& os, Color c);
4239

4340
CPPCHECKLIB std::string toString(Color c);
4441

42+
extern CPPCHECKLIB bool gDisableColors; // for testing
43+
4544
#endif

‎lib/errorlogger.cpp‎

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

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-
}
597+
static void replaceColors(std::string& source, bool erase) {
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> substitutionMapReplace =
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+
static const std::unordered_map<std::string, std::string> substitutionMapErase =
611+
{
612+
{"{reset}", ""},
613+
{"{bold}", ""},
614+
{"{dim}", ""},
615+
{"{red}", ""},
616+
{"{green}", ""},
617+
{"{blue}", ""},
618+
{"{magenta}", ""},
619+
{"{default}", ""},
620+
};
621+
if (!erase)
622+
replace(source, substitutionMapReplace);
623+
else
624+
replace(source, substitutionMapErase);
626625
}
627626

628627
std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const
@@ -928,16 +927,16 @@ std::string replaceStr(std::string s, const std::string &from, const std::string
928927
return s;
929928
}
930929

931-
void substituteTemplateFormatStatic(std::string& templateFormat, bool enableColors)
930+
void substituteTemplateFormatStatic(std::string& templateFormat, bool eraseColors)
932931
{
933932
replaceSpecialChars(templateFormat);
934-
replaceColors(templateFormat, enableColors);
933+
replaceColors(templateFormat, eraseColors);
935934
}
936935

937-
void substituteTemplateLocationStatic(std::string& templateLocation, bool enableColors)
936+
void substituteTemplateLocationStatic(std::string& templateLocation, bool eraseColors)
938937
{
939938
replaceSpecialChars(templateLocation);
940-
replaceColors(templateLocation, enableColors);
939+
replaceColors(templateLocation, eraseColors);
941940
}
942941

943942
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, bool enableColors = false);
291+
CPPCHECKLIB void substituteTemplateFormatStatic(std::string& templateFormat, bool eraseColors = false);
292292

293293
/** replaces the static parts of the location template **/
294-
CPPCHECKLIB void substituteTemplateLocationStatic(std::string& templateLocation, bool enableColors = false);
294+
CPPCHECKLIB void substituteTemplateLocationStatic(std::string& templateLocation, bool eraseColors = 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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ 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;
3435

3536
options args(argc, argv);
3637

‎test/testcolor.cpp‎

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

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

0 commit comments

Comments
 (0)