SergiusTheBest / plog

Portable, simple and extensible C++ logging library
MIT License
2.21k stars 391 forks source link

\r\n for TxtFormatter in raw consoles #269

Open davidfiala opened 1 year ago

davidfiala commented 1 year ago

For terminals in raw mode, a \n alone does not reset the character position to column 0. Modifying TxtFormatter.h to optionally accept a bool for characterReturn could fix this.

I'd be happy to submit a PR, but wanted to check if you are open to accepting this type of patch given it is probably relatively rare.

SergiusTheBest commented 1 year ago

Oh, that's interesting. What's the best way to test it?

davidfiala commented 1 year ago
#include <stdio.h>
#include <termios.h>
#include <unistd.h>

#include <plog/Formatters/TxtFormatter.h>
#include <plog/Initializers/ConsoleInitializer.h>
#include <plog/Log.h>

static plog::ColorConsoleAppender<plog::TxtFormatter> consoleAppender(plog::streamStdErr);

struct termios original_attributes;

void enable_raw_mode() {
  tcgetattr(STDIN_FILENO, &original_attributes);
  struct termios raw = {0};
  cfmakeraw(&raw);
  tcsetattr(STDIN_FILENO, TCSAFLUSH, &raw);
}

void disable_raw_mode() {
  tcsetattr(STDIN_FILENO, TCSAFLUSH, &original_attributes);
}

int main() {
  plog::init(plog::verbose, &consoleAppender);

  enable_raw_mode();

  bool forceCharacterReturn = false;

  while (1) {
    char c;

    // To test: type some text and press 't' to toggle between forcing an extra \r in to the logging statement.
    // Press 'q' to quit. (Cannot use ctrl+c because we are in raw mode.)

    if (read(STDIN_FILENO, &c, 1) == 1) {
      if (c == 't') {
        forceCharacterReturn = !forceCharacterReturn;
      }
      PLOG_INFO << "c=" << c << " forceCR=" << forceCharacterReturn << (forceCharacterReturn ? "\r" : "");
      if (c == 'q') {
        break;
      }
    }
  }

  disable_raw_mode();

  return 0;
}

type: a, b, c, t, d, e, f, q

t toggles on injecting a CR q quits

$ g++ -I/repos/plog/include example.cc

$ ./a.out
2023-09-22 12:00:11.594 INFO  [507366] [main@41] c=a forceCR=0
                                                              2023-09-22 12:00:11.783 INFO  [507366] [main@41] c=b forceCR=0
    2023-09-22 12:00:12.043 INFO  [507366] [main@41] c=c forceCR=0
                                                                  2023-09-22 12:00:13.537 INFO  [507366] [main@41] c=t forceCR=1
2023-09-22 12:00:14.198 INFO  [507366] [main@41] c=d forceCR=1
2023-09-22 12:00:14.309 INFO  [507366] [main@41] c=e forceCR=1
2023-09-22 12:00:14.387 INFO  [507366] [main@41] c=f forceCR=1
2023-09-22 12:00:19.802 INFO  [507366] [main@41] c=q forceCR=1

There's multiple cases, I think, where \n is translated to \r\n. https://man7.org/linux/man-pages/man3/termios.3.html including even O_POST.

I'd probably recommend against testing this with terminals though, since CR support might be useful for other cases such as Windows-style output or other systems that just happen to expect \r\n. It just happened to be that I noticed this with raw terminal mode enabled.

SergiusTheBest commented 1 year ago

I think PLOG_INFO << "first line \n second line" is expected to output 2 lines if raw mode support is properly implemented, right? So it's not only about \n at the end but also in the middle?

davidfiala commented 1 year ago

Correct. But I could argue that we shouldn't rewrite the user's log text itself. That could produce unexpected results if users think they are getting verbatim output, and it may require another scan through memory plus additional logic to inject the \r midstream.

FWIW, I'd think of this more as supporting environments that just happen to expect a \r on a per-log-line basis, rather than thinking of it as 'raw mode support'.