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

StringFormatters output newlines #285

Closed
haf opened this issue Mar 19, 2018 · 13 comments
Closed

StringFormatters output newlines #285

haf opened this issue Mar 19, 2018 · 13 comments
Assignees

Comments

@haf
Copy link
Member

haf commented Mar 19, 2018

/cc @adamchester @lust4life

Question for you: Logary/Formatting/StringFormatter.LevelDatetimePathMessageNl complex data

Outputs a huge chunk

It's safe to say the literate console is the thing people will debug with. Then: the string formatters should output single lines... Could you guys have a look at the tests for formatting, make sure they work across different Thread.CurrentCulture values and perhaps make them all (unless specified otherwise) output a single line with the data?

@adamchester
Copy link
Contributor

Just confirming ... the formatters, when combined together, sometimes output too much data. So we will try to ensure they’re limited to a single line. @haf do you have a sample you can paste here that shows “too much data” and/or too many lines?

@haf
Copy link
Member Author

haf commented Mar 20, 2018

I think we have two use-cases:

  • debug logging, which I think LiterateConsole is better at
    • debug logging single line mode: can literate console be configured for this? Should be default for libraries that replace Facade with the literate console (should see no difference in output and the Facade outputs single lines now)
    • debug logging expanded with all fields/props/context/exns: basically like it is now?
  • console logging for e.g. systemctl/journalctl type of logs with a log-per-line: which I'm asking for here; this is the reason the above string formatter ends with Nl, as in new-line, to be specific that it doesn't output more than a single line. "normal console logging" should then be enabled at >= Warn levels in production and always log single lines. It could output more data than is in the message.value field though; as long as it's a single line.

haf added a commit that referenced this issue Mar 23, 2018
haf added a commit that referenced this issue Mar 23, 2018
@haf haf closed this as completed Mar 29, 2018
@haf haf reopened this Mar 29, 2018
@haf
Copy link
Member Author

haf commented Mar 29, 2018

Sorry, it's not this issue that is fixed. The tests are passing, but the newline based formatters should still not print with newlines them for the console.

@lust4life
Copy link
Contributor

@haf
LevelDatetimePathMessageNl will output context value , the default format for context will output newline to show the complex context, unless user implement their own format for context.

the message.value with fields will always try to output a single line, unless user define their message template use origin string capture like "some {data:l}" , and the data ToString method output newline. otherwise the tokeniseTemplateWithGauges will output a single line.

user can easily provider their own MessageWriter to decide which part of message they want to format. And i think we can use a flag to decide whether or not to show the context at let expanded highlightError nl ending: MessageWriter = there (levelDatetimeMessagePathNewLine use this)

what do you think ?

@haf
Copy link
Member Author

haf commented Apr 5, 2018

I think that for non-literate console logging which is what LevelDateTimePathMessageNl was for we should by default output only single lines.

For anything else, I think we should leave it like this and show how to use MessageWriter with the targets.

@lust4life
Copy link
Contributor

#292 does this help ?

@haf haf closed this as completed in b55236e Apr 5, 2018
@haf
Copy link
Member Author

haf commented Apr 19, 2018

I'm finding that I'm still missing docs for how to output a single line with the literate console.

@haf haf reopened this Apr 19, 2018
@lust4life
Copy link
Contributor

@haf misunderstanding your needs before. for literate console.
https://github.com/logary/logary/blob/master/src/Logary/Targets/Core.fs#L208-L209

@haf
Copy link
Member Author

haf commented Apr 20, 2018

It's neat, but I think a simple boolean to include this (default to true) would be great, actually. /cc @adamchester

@adamchester
Copy link
Contributor

Yeah actually I made an attempt at creating one last night, but lots has changed so I got side tracked often. I think that’s the basic idea, just make a replacement for that function.

I think the default for literate is good readability for a human. I think humans would want a new line separating exceptions and maybe other common elements, right?

@haf
Copy link
Member Author

haf commented Apr 21, 2018

@adamchester Yes that is true; but in my use-case I need to debug a pretty verbosely logging service and the extra lines makes that impossible; the terminal can't hold all the text and most of the time of the developer goes to scrolling up and down and not finding what they want. That said; the extra space comes from the context always being printed and not from the exceptions. Perhaps we provide three variants; context + exn + line, exn + line and line-only + exn message?

@adamchester
Copy link
Contributor

I agree @haf.

I think we should introduce a similar concept which the Facade got first already in #234 and #247 (i.e. a customisable output template with built-in fields like {properties}, {exception}, and so on...). I think this output template feature is much easier to use than composing your own tokenise function.

I'm not sure when I would have time to tackle this myself, it could be days, weeks, months, or years away, but it is something I want to do.

In the short term, I think we have inadvertantly prevented people from copying the built-in tokenise function with some internals, so we can make those public somehow, and also expose a few pre-built functions like you have suggested which cover the most common use cases out of the box.

@haf haf closed this as completed in 3a9beff Apr 25, 2018
@haf
Copy link
Member Author

haf commented Apr 25, 2018

Done in beta.4.

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

3 participants