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

Use native console levels. #8

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

anp
Copy link

@anp anp commented Aug 23, 2020

Allows using native devtools controls for choosing what shows up, inheriting coloring from levels.

Open questions:

  • should this be another config flag instead of piggy-backing on the "use colors" flag? should it be the only option instead? (wasm-bindgen-test is rather unhappy with the css, i'll note)
  • should the "native console" levels do anything different with TRACE level events? should the console include trace-level events at all?
  • should the native console levels still include the level substring? most browsers do styling of those differently so we could maybe save some horizontal space by omitting them (as it is in this PR)

Allows using native devtools controls for choosing what shows up.
@colelawrence
Copy link
Member

Thanks for the added thoughts, @anp.

There have been several suggestions for adding higher customizability at this point (like in #3).

I'm almost wondering if we should open up the API to be much more customizable through things like allowing you to replace the default formatter (like a FnMut(EventInfo) -> Vec<String> :: event info -> console arguments), replace the default performance API caller, etc.

What do you think?

@colelawrence
Copy link
Member

Super cool addition, by the way! I think this makes a lot of sense, I may add a configuration for this behavior to opt-out.

@anp
Copy link
Author

anp commented Aug 27, 2020

Makes sense, yeah. What do you think about setting things up as Layers?

Splitting the one layer into PerformanceLayer and ConsoleLayer would make it easy to have different initialization mechanisms. You could maybe have a PerformanceLayer::from_global() and a PerformanceLayer::from_instance(p: js_sys::...)? Similarly there could be ConsoleLayer::info() and ConsoleLayer::with_native_levels(). Or perhaps builders for the layers?

You could still have convenience functions like set_as_global_default on top of course.

@anp
Copy link
Author

anp commented Aug 27, 2020

And yeah I think that having progressive levels of customization is very valuable and in line with tracing's philosophy AFAICT.

@colelawrence
Copy link
Member

@anp, awesome suggestion! I really like the idea of splitting them up as layers, and having a default set of layers for different environments.

I can look at doing this next week.

@@ -122,7 +139,14 @@ impl<S: Subscriber + for<'a> LookupSpan<'a>> Layer<S> for WASMLayer {
"color: inherit",
);
} else {
log1(&format!("{} {}{}", level, origin, recorder));
let output = format!("{}{}", origin, recorder);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above here, it looks like we could still wire up log4, info4, debug4, etc

@colelawrence
Copy link
Member

Notes for the future: if this is continued to be implemented, I would like it to be guarded by a config option

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

Successfully merging this pull request may close these issues.

2 participants