Command Line coloring by morm24 · Pull Request #131 · cbielow/OpenMS

morm24 · 2022-04-29T13:04:01Z

Description

Added an interface to enable coloring the console output.
Important are the classes Colorizer (Colorizer.ccp & Colorizer.h).
Changes in TOPPBasse and other classes are just for testing and not final in any way right now.

Checklist:

Make sure that you are listed in the AUTHORS file
[-] Add relevant changes and new features to the CHANGELOG file
I have commented my code, particularly in hard-to-understand areas
[-] New and existing unit tests pass locally with my changes
[-] Updated or added python bindings for changed or new classes. (Tick if no updates were necessary.)

How can I get additional information on failed tests during CI:

If your PR is failing you can check out

http://cdash.openms.de/index.php?project=OpenMS and look for your PR. If you click in the column that lists the failed tests you will get detailed error messages.
Or click on the action: e.g., for clang-format linting

Note:

Once you opened a PR try to minimize the number of pushes to it as every push will trigger CI (automated builds and test) and is rather heavy on our infrastructure (e.g., if several pushes per day are performed).

Advanced commands (admins / reviewer only):

/reformat (experimental) applies the clang-format style changes as additional commit
setting the label "NoJenkins" will skip tests for this PR on jenkins (saves resources e.g., on edits that do not affect tests)
commenting with rebuild jenkins will retrigger Jenkins-based CI builds

…for colorizing command line output (on Linux) is esablished

…Colorizer Objects. (enables colorizer calls in functions with std::string expection)

lennykovac

Looks good overall but stick to the coding-convention and a lot of unnecessary spaces/newlines.
Try to stick to english-comments :)

lennykovac · 2022-05-02T08:06:07Z

src/openms/source/CONCEPT/Colorizer.cpp

+    //If we are on windows, get the "default" console color and safe it in _defcolor
+    #if defined(_WIN32) || defined(_WIN32) || defined(__WIN32__) || defined(_WIN64)
+    CONSOLE_SCREEN_BUFFER_INFO Info;
+    HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);


weird var-name

Suggested change

HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);

HANDLE std_out_handler = GetStdHandle(STD_OUTPUT_HANDLE);

src/openms/include/OpenMS/CONCEPT/Colorizer.h

src/openms/source/CONCEPT/Colorizer.cpp

hbeschorner · 2022-05-02T07:30:17Z

src/openms/include/OpenMS/CONCEPT/Colorizer.h

+
+
+
+    template <typename T = std::string>


why a template if it's always a string

its not always a string, the default valueis a string. It has to accept all types, that work with the << operator.
I had to define the default type allready in the Template. Just writing the default in the Attributes wouldn't be accepted.

src/openms/include/OpenMS/CONCEPT/Colorizer.h

+      this->_input.str("");
+      this->_input << s;


src/openms/include/OpenMS/CONCEPT/Colorizer.h

hbeschorner · 2022-05-02T08:01:16Z

src/openms/source/APPLICATIONS/TOPPBase.cpp

                    "Either provide a full filepath or fix your PATH environment!" +
                    (p.required ? "" : " Since this file is not strictly required, you might also pass the empty string \"\" as "
-                    "argument to prevent its usage (this might limit the usability of the tool)."));
+                    "argument to prevent it's usage (this might limit the usability of the tool)."));


pretty sure its was correct

ddint know I changed it. yes, old one is correct

src/openms/source/CONCEPT/Colorizer.cpp

src/openms/include/OpenMS/CONCEPT/Colorizer.h

cbielow · 2022-05-02T10:13:31Z

src/openms/include/OpenMS/CONCEPT/Colorizer.h

+    /// Copy constructor
+    //Colorizer(const Colorizer &rhs);


make explicit
= delete
or
= default

cbielow · 2022-05-02T10:53:34Z

src/openms/source/CONCEPT/Colorizer.cpp

+    CONSOLE_SCREEN_BUFFER_INFO Info;
+    HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);
+    GetConsoleScreenBufferInfo(hStdout, &Info);
+    _defcolor = Info.wAttributes;


remember two default colors, for cerr and cout

cbielow · 2022-05-02T11:00:54Z

src/openms/source/CONCEPT/Colorizer.cpp

+    //If we are on windows, get the "default" console color and safe it in _defcolor
+    #if defined(_WIN32) || defined(_WIN32) || defined(__WIN32__) || defined(_WIN64)
+    CONSOLE_SCREEN_BUFFER_INFO Info;
+    HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);
+    GetConsoleScreenBufferInfo(hStdout, &Info);
+    _defcolor = Info.wAttributes;


move this to another class (construction + destrcution)

namespace OpenMS { namespace Internal { struct WindowsOSDefaultColor { WindowsOSDefaultColor() { std::cout << "saving default colors...\n"; /// get and remember 2 default colors } ~WindowsOSDefaultColor() { std::cout << "restoring default colors...\n"; } int default_cerr_; int default_cout_; }; // end WindowsOSDefaultColor extern static const WindowsOSDefaultColor default_color____; } // internal } // OpenMS

cbielow · 2022-05-02T11:03:09Z

src/openms/source/CONCEPT/Colorizer.cpp

+  auto Colorizer::getColor(int i)
+  {
+    //checking if parameter is outside array size or not set at all
+    // Darf nicht NULL, da dadurch kein "black" mehr möglich ist
+    return ((i == -1 || i > 8 || i < 0) ? this->colors[this->_color] : this->colors[i]);
+  }
+
+  std::string Colorizer::getText()
+  { 
+
+    return _input.str();
+  }   


cbielow · 2022-05-02T11:04:07Z

src/openms/source/CONCEPT/Colorizer.cpp

+  // overload the shift operator (<<)
+  std::ostream &operator<<(std::ostream &o_stream, OpenMS::Colorizer& col)


Suggested change

// overload the shift operator (<<)

std::ostream &operator<<(std::ostream &o_stream, OpenMS::Colorizer& col)

// overload the shift operator (<<)

std::ostream &operator<<(std::ostream &o_stream, OpenMS::Colorizer& col)

{

col.outputToStream(o_stream);

return o_stream;

}

cbielow · 2022-05-02T11:04:43Z

src/openms/source/CONCEPT/Colorizer.cpp

+    // colorize string with color set in the object
+#if defined(_WIN32) || defined(_WIN32) || defined(__WIN32__) || defined(_WIN64)
+
+    //set color of output   
+    SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE),col.getColor());
+
+    // paste text
+    o_stream << col.getText();
+
+    // recover old Console font and set it as new one.
+    if(col.getReset()) 
+    {
+    SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), col._defcolor);


move to Colorizer::outputToStream(std::ostream&);

src/openms/source/APPLICATIONS/TOPPBase.cpp

Changes made in pull request code review. Not working, will be fixed by next commit. Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de> Co-authored-by: Lenny Kovac <44415793+lennykovac@users.noreply.github.com> Co-authored-by: hbeschorner <102038306+hbeschorner@users.noreply.github.com>

…nality will be set with next commit

…olor inside function call

…piling.

…verbose).

…rizer.

…ows not tested). Basics of IndentedStringStream implemented.

…breakstring func. keine Testklassen

src/openms/include/OpenMS/APPLICATIONS/ConsoleUtils.h

src/openms/include/OpenMS/CONCEPT/Colorizer.h

src/openms/source/APPLICATIONS/ConsoleUtils.cpp

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

Moritz Berger and others added 9 commits April 14, 2022 10:48

Added class and header Colorizer.* into concept. Basic functionality …

52d7559

…for colorizing command line output (on Linux) is esablished

Merge branch 'OpenMS:develop' into coloring

e7ab5b1

updated colorizer class

11df650

Coloring aktualisiert, lokases verzeichnis auf neusten stand gebracht.

38b5ec6

added full functionality for windows use

bfbad5e

added colorizer to sources.cmake

04ea4f4

colorizer in TOPPBase hinzugefügt, Testcodes eingefügt

c5c3447

changed to colorizing functions, with std::string output rather than …

87d41b7

…Colorizer Objects. (enables colorizer calls in functions with std::string expection)

added few comments, clead functions, ready for Pull request (overview)

2eb89f1

morm24 changed the title ~~COmmand Line coloring~~ Command Line coloring Apr 29, 2022

cbielow changed the base branch from develop to AC_DFA April 29, 2022 13:28

cbielow changed the base branch from AC_DFA to develop April 29, 2022 13:29

lennykovac reviewed May 2, 2022

View reviewed changes

hbeschorner reviewed May 2, 2022

View reviewed changes

cbielow requested changes May 2, 2022

View reviewed changes

morm24 commented May 2, 2022

View reviewed changes

src/openms/source/APPLICATIONS/TOPPBase.cpp Outdated Show resolved Hide resolved

morm24 and others added 10 commits May 2, 2022 13:28

changed pull request changes, now working with linux. windows functio…

5749fa6

…nality will be set with next commit

corrected include path, addition to last commit: removed ability to c…

5a32c13

…olor inside function call

Error in windows functionality: struct WindowsOSDefaultColor when com…

90b30ac

…piling.

struct error should be fixed now.

de98618

updatetd colorizer.cpp include path

19cb2e5

static WindowsOSDefaultColor struct now working under Windows (still …

e58e558

…verbose).

work in progress, started changing the Console Utils, tidying up Colo…

9af09bf

…rizer.

Colorizer functionality now working again and tested with linux (wind…

ec0508c

…ows not tested). Basics of IndentedStringStream implemented.

kompiliert, fehler in FOrmatierung, und zugriffsfehler innerhalb der …

ef356a5

…breakstring func. keine Testklassen

cbielow reviewed May 11, 2022

View reviewed changes

morm24 and others added 3 commits May 12, 2022 11:47

Update src/openms/include/OpenMS/APPLICATIONS/ConsoleUtils.h

c76c8de

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

Update src/openms/include/OpenMS/APPLICATIONS/ConsoleUtils.h

61a9b7d

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

Update src/openms/include/OpenMS/CONCEPT/Colorizer.h

bf33585

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

morm24 and others added 3 commits May 12, 2022 11:51

Update src/openms/source/APPLICATIONS/ConsoleUtils.cpp

9d833a7

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

Update src/openms/source/APPLICATIONS/ConsoleUtils.cpp

5af7fa1

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

Update src/openms/source/APPLICATIONS/ConsoleUtils.cpp

40835d8

Co-authored-by: Chris Bielow <chris.bielow@fu-berlin.de>

	HANDLE hStdout = GetStdHandle(STD_OUTPUT_HANDLE);
	HANDLE std_out_handler = GetStdHandle(STD_OUTPUT_HANDLE);

		// overload the shift operator (<<)
		std::ostream &operator<<(std::ostream &o_stream, OpenMS::Colorizer& col)

Conversation

morm24 commented Apr 29, 2022

Description

Checklist:

How can I get additional information on failed tests during CI:

Note:

Advanced commands (admins / reviewer only):

Uh oh!

lennykovac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lennykovac left a comment •

edited

Loading