Allow to provide custom levels and custom colors #257

matteozambon89 · 2021-11-02T16:32:33Z

This should provide the possibility to handle custom levels and custom colors, especially through CLI.

package.json

lib/colors.js

bin.js

mcollina

lgtm

mcollina

tests must be re-enabled

matteozambon89 · 2021-11-16T12:00:26Z

@mcollina all done! Let me know if you can merge and deploy it please

jsumners

Please update documentation, including the Readme.

lib/colors.js

lib/utils.js

mcollina

lgtm

matteozambon89 · 2021-12-01T15:53:43Z

@mcollina and @jsumners can this be merged to master?

jsumners

Can you perform a rebase?

$ git remote add upstream https://github.com/pinojs/pino-pretty.git
$ git fetch upstream master
$ git rebase upstream/master

package.json

matteozambon89 · 2021-12-08T15:08:38Z

@jsumners I've removed package.json from my commits as the changes shouldn't have been committed in the first place and rebased to pinojs:master once again.

jsumners

Looks good to me.

mcollina

lgtm

mcollina · 2021-12-09T08:49:42Z

can you check CI? It was failing

dotcore · 2022-01-19T13:43:59Z

I am also very interested in this feature, so I looked into why the CI is failing. For some probably non-intentional reason in commit id c94f89c in file test/basic.test.js, following code gets changed to:

    const expectedLines = [
      '    msg: {',
      '      "b": {',
      '        "c": "d"',
      '      },',
      '      "a": "[Circular ~]"',
      '    }'
    ]

instead of:

    const expectedLines = [
      '    msg: {',
      '      "a": "[Circular]",',
      '      "b": {',
      '        "c": "d"',
      '      }',
      '    }'
    ]

After that the tests are all green.
I think this also could use a rebase to upstream again. I could also submit a pull request, if you want.

matteozambon89 · 2022-01-19T14:16:26Z

I am also very interested in this feature, so I looked into why the CI is failing. For some probably non-intentional reason in commit id c94f89c in file test/basic.test.js, following code gets changed to:
    const expectedLines = [
      '    msg: {',
      '      "b": {',
      '        "c": "d"',
      '      },',
      '      "a": "[Circular ~]"',
      '    }'
    ]
instead of:
    const expectedLines = [
      '    msg: {',
      '      "a": "[Circular]",',
      '      "b": {',
      '        "c": "d"',
      '      }',
      '    }'
    ]
After that the tests are all green. I think this also could use a rebase to upstream again. I could also submit a pull request, if you want.

Sorry if I've been AWOL, between Xmas holidays and New Year "back to work" I haven't had time to look into this issue.

Thanks @dotcore for your input. The commit was made somewhat intentionally as the local test was failing and wasn't allowing me to push. I'll revert that commit and push again to see if it fixes the issue.

I will also do a rebase to upstream, so we can hopefully close this PR.

Allow to override message, greyMessage colors

matteozambon89 · 2022-01-19T14:31:56Z

@mcollina @jsumners @dotcore reverted the test changes and rebased to upstream

dotcore · 2022-01-19T14:55:54Z

@mcollina @jsumners @dotcore reverted the test changes and rebased to upstream

One typo is left.

' "a": "[Circular]",',

instead of

' "a": "[Circular],"',

This causes 1 test to fail.

matteozambon89 · 2022-01-19T15:07:56Z

@dotcore I've updated that, although my tests keep failing for the test/basic.test.js as the format is different on my machine.

I'm on MacOS with node 16, which environment are you running the tests from?

dotcore · 2022-01-19T15:16:36Z

@dotcore I've updated that, although my tests keep failing for the test/basic.test.js as the format is different on my machine.

I'm on MacOS with node 16, which environment are you running the tests from?

I'm on MacOS with node 17.4.0, but I also tested on node 16.13.2 and 12.22.9 (Switched with nvm).

matteozambon89 · 2022-01-19T15:19:31Z

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

> [email protected] test
> tap --100 --color

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected              
  +++ actual                
  @@ -1,1 +1,1 @@           
  -      "a": "[Circular]", 
  +      "b": {             

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected      
  +++ actual        
  @@ -1,1 +1,1 @@   
  -      "b": {     
  +        "c": "d" 

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected      
  +++ actual        
  @@ -1,1 +1,1 @@   
  -        "c": "d" 
  +      },         

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected               
  +++ actual                 
  @@ -1,1 +1,1 @@            
  -      }                   
  +      "a": "[Circular ~]" 

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                                             
  931 |     Object.assign(fs, { close, writeSync })                              
  932 |     t.match(chunks.join(''), /INFO .+: this message has been buffered/)  
> 933 |     t.equal(closeCalled, false)                                          
      | ------^                                                                  
  934 |   })                                                                     
  935 |                                                                          
  936 |   t.test('stream usage', async (t) => {                                  

  test: test/basic.test.js basic prettifier tests does not call fs.close on stdout
    stream
  found: true
  wanted: false
  compare: ===
  stack: |
    Test.<anonymous> (test/basic.test.js:933:7)
    Test.<anonymous> (test/basic.test.js:910:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

dotcore · 2022-01-19T15:27:12Z

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

I can confirm that it works for me, also CI didn't fail. No idea why it does not work for you.

matteozambon89 · 2022-01-19T15:33:47Z

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

I can confirm that it works for me, also CI didn't fail. No idea why it does not work for you.

Who knows 😂

dotcore · 2022-01-19T15:36:27Z

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

I can confirm that it works for me, also CI didn't fail. No idea why it does not work for you.

Who knows 😂

Not sure if this may help, but maybe delete your node_modules and run npm i, in case you have some outdated dependencies.

matteozambon89 · 2022-01-20T10:14:07Z

@dotcore nope, that didn't solve the issue. Something between node and MacOS must be configured differently in this machine.

@mcollina can we have this PR merged to master?

mcollina · 2022-01-20T17:47:57Z

Done

mcollina reviewed Nov 2, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

lib/colors.js Outdated Show resolved Hide resolved

matteozambon89 commented Nov 2, 2021

View reviewed changes

bin.js Outdated Show resolved Hide resolved

mcollina approved these changes Nov 4, 2021

View reviewed changes

mcollina requested changes Nov 4, 2021

View reviewed changes

matteozambon89 requested a review from mcollina November 16, 2021 12:02

jsumners requested changes Nov 16, 2021

View reviewed changes

lib/colors.js Outdated Show resolved Hide resolved

lib/colors.js Show resolved Hide resolved

lib/utils.js Outdated Show resolved Hide resolved

lib/utils.js Outdated Show resolved Hide resolved

mcollina approved these changes Nov 16, 2021

View reviewed changes

matteozambon89 force-pushed the custom-log-levels branch from 5a14c5a to 8d328c0 Compare December 1, 2021 15:52

jsumners requested changes Dec 4, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

matteozambon89 force-pushed the custom-log-levels branch 4 times, most recently from 3186519 to c94f89c Compare December 8, 2021 15:06

jsumners approved these changes Dec 8, 2021

View reviewed changes

mcollina approved these changes Dec 9, 2021

View reviewed changes

matteozambon89 added 8 commits January 19, 2022 14:29

Allow to provide custom levels and custom colors

e98b7db

Allow log levels with 0 value

e991ad1

Fixed linting

5cc5974

Introduced factory instead of signleton override for customColors

0466ca0

Use one-char x/X for custom level/colors

a474d11

Enabled tests and fixed coverage to 100%

5fb4dce

Allow to override message, greyMessage colors

Rebase and requested fixes

9f87532

Reverted c94f89c

871b516

matteozambon89 force-pushed the custom-log-levels branch from c94f89c to 871b516 Compare January 19, 2022 14:29

Fixed issue with trailing comma

24fa561

mcollina merged commit 0c3993a into pinojs:master Jan 20, 2022

matteozambon89 deleted the custom-log-levels branch January 21, 2022 09:57

deepcrawltech mentioned this pull request Feb 5, 2022

chore(deps): update all dependencies deepcrawl/graphql-sample-project#19

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to provide custom levels and custom colors #257

Allow to provide custom levels and custom colors #257

matteozambon89 commented Nov 2, 2021 •

edited

Loading

mcollina left a comment

mcollina left a comment

matteozambon89 commented Nov 16, 2021

jsumners left a comment

mcollina left a comment

matteozambon89 commented Dec 1, 2021

jsumners left a comment

matteozambon89 commented Dec 8, 2021 •

edited

Loading

jsumners left a comment

mcollina left a comment

mcollina commented Dec 9, 2021

dotcore commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022 •

edited

Loading

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022 •

edited

Loading

matteozambon89 commented Jan 20, 2022

mcollina commented Jan 20, 2022

Allow to provide custom levels and custom colors #257

Allow to provide custom levels and custom colors #257

Conversation

matteozambon89 commented Nov 2, 2021 • edited Loading

mcollina left a comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

matteozambon89 commented Nov 16, 2021

jsumners left a comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

matteozambon89 commented Dec 1, 2021

jsumners left a comment

Choose a reason for hiding this comment

matteozambon89 commented Dec 8, 2021 • edited Loading

jsumners left a comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

mcollina commented Dec 9, 2021

dotcore commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022 • edited Loading

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022

matteozambon89 commented Jan 19, 2022

dotcore commented Jan 19, 2022 • edited Loading

matteozambon89 commented Jan 20, 2022

mcollina commented Jan 20, 2022

matteozambon89 commented Nov 2, 2021 •

edited

Loading

matteozambon89 commented Dec 8, 2021 •

edited

Loading

dotcore commented Jan 19, 2022 •

edited

Loading

dotcore commented Jan 19, 2022 •

edited

Loading