pinojs / pino-pretty

🌲Basic prettifier for Pino log lines
MIT License
1.27k stars 150 forks source link

hideObject option does not enforce line breaks? #446

Closed timlohse1104 closed 1 year ago

timlohse1104 commented 1 year ago

It seems that hideObject option does not implement line breaks.

Without the option toggled on logs look like this:

[26.7.2023, 11:18:37] INFO: [Nest] 26.7.2023, 11:18:37 [RoutesResolver] | HealthCheckController {/ocr/health}: {"context":"RoutesResolver"} [26.7.2023, 11:18:37] INFO: [Nest] 26.7.2023, 11:18:37 [RouterExplorer] | Mapped {/ocr/health/metrics, GET} route {"context":"RouterExplorer"} [26.7.2023, 11:18:37] INFO: [Nest] 26.7.2023, 11:18:37 [RouterExplorer] | Mapped {/ocr/health/check, GET} route {"context":"RouterExplorer"}

hideObject: true transforms these logs into a single line log:

[26.7.2023, 11:18:37] INFO: [Nest] 26.7.2023, 11:18:37 [RoutesResolver] | HealthCheckController {/ocr/health}: [26.7.2023, 11:18:37] INFO: [Nest] 26.7.2023, 11:18:37 [RouterExplorer] | Mapped {/ocr/health/metrics, GET} route [26.7.2023, 11:18:37] INFO: [Nest] 26.7.2023, 11:18:37 [RouterExplorer] | Mapped {/ocr/health/check, GET} route

Is this a bug or a feature?

jsumners commented 1 year ago

It was implemented by https://github.com/pinojs/pino-pretty/pull/145

timlohse1104 commented 1 year ago

Looks like this bug originated from this implementation. After ìf (!hideObject there is just an return of the current line. https://github.com/pinojs/pino-pretty/blob/fa386a964cce7d7c5cbeb4ce5bb8d28785001acd/lib/utils.js#L372 added the eol in the past but there need to be another eol in https://github.com/pinojs/pino-pretty/blob/master/index.js#L202C38-L202C54 and

// In single line mode, include a space only if prettified version isn't empty
      if (singleLine && !/^\s$/.test(prettifiedObject)) {
        line += ' '
      }
      line += prettifiedObject

could be moved inside the hideObject condition.

timlohse1104 commented 1 year ago

Should i provide you with a pull request?

mcollina commented 1 year ago

yes please!

Totalus commented 1 year ago

Is that fixed ?

mcollina commented 1 year ago

Is that fixed ?

Doesn't look like it. Would you like to send a PR?

timlohse1104 commented 1 year ago

I tried to recreate this behavior but was not able to do so. I think this issue can be closed.