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

hs.logger.log should render objects with hs.inspect, not just lua print #2865

Open
slippycheeze opened this issue May 6, 2021 · 3 comments

Comments

@slippycheeze
Copy link

slippycheeze commented May 6, 2021

why? it is useful, when debugging, to include larger lua objects. the logger, internally, does nothing if the log level is higher than debug, which is great.

to render tables, etc, though, someone needs to use a pretty-printer on them. if I do that to the argument then I'm rendering a table to a string which is then completely ignored, and that is kind of not cool. especially if I'm in the context of a high frequency callback.

if the logger did that internally, though, it could delay that work until later: when it decides the event should be logged, it then spends the time turning the object into a string. that way I don't need to think about the "cost" of dumping an object so much.

I can implement this manually, by testing the logger level by hand—but that required poking my nose into the implementation to figure out how to ask "is the logger going to do something with a debug message?"

current behaviour:

> hs.logger.new('test').w(1, 2, {a=1,b=2})
13:20:27 ** Warning:      test: 1 2 table: 0x6000017a88c0

desired behaviour:

13:36:58 ** Warning:      test: 1 2 {
  a = 1,
  b = 2
}

or better, but not really in reach with the upstream library:

13:36:58 ** Warning:      test: 1 2 {a = 1, b = 2}
@asmagill
Copy link
Member

asmagill commented May 6, 2021

FWIW, you can approximate your second example suggestion with hs.inspect(tbl, { newline = " ", indent = "" }).

I agree that this would make logger more useful with tables. I would like to point out that there are a few cases where tables are used like userdata and already have a __tostring method, so there should probably be some logic like:

...
    if type(item) == "table" then
        if (getmetatable(item) or {}).__tostring then
            -- use tostring(item)
        else
            -- use hs.inspect(item, {  newline = " ", indent = "" })
        end
    end
...

@latenitefilms
Copy link
Contributor

I think if this change is made, it would annoy me, as a lot of the time I use hs.logger to double check the type of something, so I actually just want to see something like table: 0x6000017a88c0 rather than printing out the contents of the table.

Maybe there could be a new method added to hs.logger so it doesn't break existing functionality? For example:

hs.logger.new('test').wT(1, 2, {a=1,b=2})

@gineer01
Copy link

This PR #2627 proposes a way to allow changing the handler for logging: you can change hs.logger to use hs.inspect or log to file if you want to.

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

No branches or pull requests

4 participants