orottier/web-audio-api-rs

Many public facing types do not implement Debug

CorvusPrudens opened this issue · 5 comments

Types such as context::online::AudioContext, GainNode, and likely others do not implement std::fmt::Debug. This makes implementing Debug for wrapping types cumbersome, and prevents the use of expect (among other things) involving these types.

I'm not sure if there are any major trade offs in terms of compile time or other metrics, but I'd personally prefer to have Debug regardless.

Thanks for the report. We should probably do this as it is also advised in the rust API guidelines https://rust-lang.github.io/api-guidelines/debuggability.html#all-public-types-implement-debug-c-debug

If you are willing to open a PR to address this that would be very welcome. You can use the #![deny(missing_debug_implementations)] lint to find all missing impls.

For some types this is easy and straightforward. For others, there are many inner library types contained within. Is a simple #[derive(Debug)] appropriate for all of these, or should some of them provide an opaque debug representation?

For example, a #[derive(Debug)] on context::online::AudioContext would require an implementation for context::concrete_base::ConcreteBaseAudioContext, and potentially all of its members.

Personally, I think it would be nice, but it could end up meaning a lot of types will have Debug derived.

Right, good question.

I think it would be best to start small. Most of the AudioContext fields have no 'debug' value anyway and only expose internal guts that are subject to change.

Take a look at the Debug impl of RenderThread and Graph for example. I'm only exposing a few fields.

Perhaps the Debug implementation of AudioContext should only expose what we provide via public method getters (base, sink_id, latency values perhaps). ConcreteBaseAudioContext should list state, sample_rate, current_time, max_channel_count, offline. AudioParam should list all internals expect registration. Etc etc

Hey @CorvusPrudens, are you working on this? Otherwise I will make a start

I have not! You certainly won't be stepping on my toes.