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

More robust log formatting for easier customization #40

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nic-hartley
Copy link

Closes #38.

TODO:

  • Write the first pass at the traits
  • Tree deconstructor (turn a &Tree into an Iterator<Item=(&[Indent], &Event)>)
  • Formatting implementation based on that
  • Handle multi-line FormatEvent outputs
  • Consider: Reducing allocations? (Easy win: format to Write instead of String)
  • Test implementations: Reimpl of Pretty, new impl of Ascii

@QnnOkabayashi
Copy link
Owner

Hi! Thanks for doing this so quickly. Looking at what you've done so far, I'm realizing my initial suggestions might not be the best idea as I see a lot of complexity coming with this.

I'm wondering if it might be a good idea to instead break down the existing Pretty impl into several composable and reusable functions that we expose publicly, which you can use to piece together the formatter you want. Thoughts on this approach instead? The main benefit here would be that other users could really easily figure out what's going on, while our current trajectory would require a lot of documentation to get others up to speed.

Let me know what you think.

@nic-hartley
Copy link
Author

@QnnOkabayashi Consider me astonished that the first idea didn't turn out to be perfect.

So at a high level what I'd like to see is an API that makes it relatively easy to write a renderer that works line-by-line, instead of having to decompose a whole Tree at once. (I guess in simple cases you could make it recurse down into the Spans, but for any complex indenting -- like what Pretty does, or how I'd want Ascii to look -- it seems like it'd get complex.) So that API could look a few ways:

// using APIs on e.g. `Tree` to implement the current trait in line-by-line terms
impl Formatter for MyFormat {
  type Error = !;
  fn fmt(&self, tree: &Tree) -> Result<String, !> {
    let mut res = String::new();
    for (indent, event) in tree.make_things() {
      // render the indentation, then the event, into the result
    }
    Ok(res)
  }
}

// making the line-by-line thing into a new trait
impl FormatLogLine for MyFormat {
  type Error = !;
  fn fmt(&self, indent: &[Indent], event: &Event) -> Result<String, !> {
    // render the indentation, then the event
  }
}

// splitting into two (or more) traits for each part of the line
impl FormatIndents for MyFormat {
  type Error = !;
  fn fmt(&self, indent: &[Indent]) -> Result<String, !> {
    // render the indentation
  }
}
impl FormatEvent for MyFormat {
  type Error = !;
  fn fmt(&self, event: &Event) -> Result<String, !> {
    // render the event
  }
}

To be clear, I don't think Formatter should go away, there's always use for that level of API (e.g. the JSON example) -- but I think there should also be another, higher abstraction level for people trying to get directly human-readable output.

I actually like the FormatLogLine trait best of the three I sketched here. I'm not sold on the Indent API (it was cribbed largely from what Pretty does, but trying to support e.g. numbering entries) but I think it'd do well enough to build a working implementation to futz around with and bikeshed some more.

Either way, Pretty would definitely want to be rewritten in terms of the new API, if only because so much code is going to be yoinked directly from Pretty, so it doesn't make sense to duplicate it.

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.

Minorly tweaking log output
2 participants