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

fixes logging of array values (issue #10) #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kgraves
Copy link

@kgraves kgraves commented Oct 9, 2015

Also adds unit test.

Thanks for this node module @TomFrost. 👍

@jeffijoe
Copy link
Contributor

jeffijoe commented Jun 9, 2016

@TomFrost are you going to merge this any time soon? :)

@wangshan
Copy link

I think I'm hit by the same issue, it's unable to log array of RegExp objects, will this patch fix it?

@kgraves
Copy link
Author

kgraves commented Sep 13, 2016

No, I don't think this will fix your issue. The problem is how JSON.stringify converts a regex to a string

$ node
> JSON.stringify(/asdf/)
'{}'

You could write a patch to handle a regex case. However, it would only work where it is a top-level object in what you are logging. For nested objects, Bristol uses JSON.stringify. Your best bet might be to use something like regex-stringify before you use Bristol. Otherwise you'll have to implement a solution where you walk the entire logged object, and find each regex.

@TomFrost
Copy link
Owner

I've been playing with this one locally. While I like what this is doing, It feels a bit offputting that the array values are logged with their numeric keys, which in the context of a flatted log message, are fairly meaningless (and can get downright confusing if you log more than one array, and Bristol uses its method of finding a non-duplicate key).

My next thought was to treat elements of an array as though they were all passed as individual values to the logging function. It worked a treat, but you lose all cohesion/relationship between those values in that process. Sometimes that's fine, but that's a bad default.

So what I've been chewing on is giving it some generic key like "array" (which can be changed by passing it in an object instead) and letting the formatters flatten that however they desire. It's actually a slightly easier integration that way too.

As for the values themselves @wangshan and @kgraves, the next release of Bristol will be 2.0, so I'm looking at refactoring all of the stringifying-logic (json or otherwise) into a shared lib to reduce the largely-repeated code in the formatters. I can certainly do a recursive scan of objects in there and properly call .toString on RegExps before JSON.stringify() gets called. Let me know if that would scratch your itch.

Thoughts are welcome!

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

Successfully merging this pull request may close these issues.

4 participants