Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

\r\n for TxtFormatter in raw consoles #269

Open
davidfiala opened this issue Sep 21, 2023 · 4 comments
Open

\r\n for TxtFormatter in raw consoles #269

davidfiala opened this issue Sep 21, 2023 · 4 comments

Comments

@davidfiala
Copy link

davidfiala commented Sep 21, 2023

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.

@davidfiala davidfiala changed the title \r \r\n for TxtFormatter in raw consoles Sep 21, 2023
@SergiusTheBest
Copy link
Owner

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

@davidfiala
Copy link
Author

#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
Copy link
Owner

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
Copy link
Author

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'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants