Skip to content

[trivial] Add newlines to end of log messages. #12887

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.

Sign up for GitHub

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

Jump to bottom
Merged
laanwj merged 1 commit into from
Apr 5, 2018
Merged

[trivial] Add newlines to end of log messages. #12887

laanwj merged 1 commit into from
Apr 5, 2018

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Apr 4, 2018

Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.

@jnewbery
[trivial] Add newlines to end of log messages.
5b10ab0 
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.
@jamesob
Copy link
Contributor

jamesob commented Apr 4, 2018

utACK 5b10ab0

Worth fixing this more generally in LogPrintf later on?

@maflcko
Copy link
Member

maflcko commented Apr 4, 2018

Mind to add a linter for this, since we have those issues every couple of months?

@jnewbery
Copy link
Contributor Author

jnewbery commented Apr 4, 2018

Mind to add a linter for this, since we have those issues every couple of months?

There are legitimate uses for this, for example to build up a log message from multiple parts:

bitcoin/src/sync.cpp

Line 83 in 1d54004

LogPrintf(" (1)");

and

bitcoin/src/txdb.cpp

Line 373 in 1d54004

LogPrintf("[0%%]...");

(I suppose those could be changed to build a string, which is then passed to LogPrintf())

@Empact
Copy link
Contributor

Empact commented Apr 5, 2018

utACK 5b10ab0

2 similar comments
@maflcko
Copy link
Member

maflcko commented Apr 5, 2018

utACK 5b10ab0

@fanquake
Copy link
Member

fanquake commented Apr 5, 2018

utACK 5b10ab0

@laanwj laanwj merged commit 5b10ab0 into bitcoin:master Apr 5, 2018
laanwj added a commit that referenced this pull request Apr 5, 2018
@laanwj
Merge #12887: [trivial] Add newlines to end of log messages.
2fc9437 
5b10ab0 [trivial] Add newlines to end of log messages. (John Newbery)

Pull request description:

  Log messages should terminate with a '\n', or the following log will be
  written to the same line without a timestamp. Fix a couple of cases
  where the message is not terminated with a \n.

Tree-SHA512: 88677afe85c88ce9f58312430e8881916bd76bbc8cd353ff81c97b3de8356680503160992c0ef3ea192b4694e848e9ca2480dbc38fea1776903b3784497f1af6
@laanwj
Copy link
Member

laanwj commented Apr 5, 2018

There are legitimate uses for this, for example to build up a log message from multiple parts:

Could have the linter check for a specific comment /* Continued */ or such.

(I suppose those could be changed to build a string, which is then passed to LogPrintf())

That would defeat the intent, to log partial lines to show progress.

@jnewbery
Copy link
Contributor Author

jnewbery commented Apr 5, 2018

Could have the linter check for a specific comment /* Continued */ or such

Yes, good idea. Longer term, it could be good to have a LogPrintf() and a LogPrintfContinuation(), and remove all \ns from the log strings. LogPrintf() would then prefix the log string with a newline followed by the timestamp. LogPrintfContinuation() would just print the log string.

That would defeat the intent, to log partial lines to show progress.

Yes, good point

@jnewbery jnewbery mentioned this pull request Apr 5, 2018
Merged
@jnewbery jnewbery deleted the log_messages_newlines branch April 5, 2018 14:36
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request May 29, 2018
@jnewbery
[trivial] Add newlines to end of log messages.
ec1d30e 
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.

Github-Pull: bitcoin#12887
Rebased-From: 5b10ab0
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jul 12, 2018
@jnewbery
[trivial] Add newlines to end of log messages.
2291774 
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.

Github-Pull: bitcoin#12887
Rebased-From: 5b10ab0
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Jan 11, 2019
@jnewbery
[trivial] Add newlines to end of log messages.
1d53e22 
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.

Github-Pull: bitcoin#12887
Rebased-From: 5b10ab0
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Utils/log/libs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jnewbery @jamesob @maflcko @Empact @fanquake @laanwj