Improve Debug impls #13
Conversation
| @@ -481,7 +492,7 @@ impl From<Vec<u8>> for Der<'static> { | |||
|
|
|||
| impl fmt::Debug for Der<'_> { | |||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | |||
| f.debug_tuple("Der").field(&self.as_ref()).finish() | |||
| hex(f, self.as_ref()) | |||
There was a problem hiding this comment.
Did you consider leaving the Der(..) wrapping in? Why/why not?
There was a problem hiding this comment.
I left this unchanged, both options seem reasonable to me.
There was a problem hiding this comment.
It winds up with multiple layers of wrapping in the Debug output, which gets kind of noisy. For instance, since CertificateDer is:
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct CertificateDer<'a>(Der<'a>);It would output as CertificateDer ( Der ( 03040506 ) ). To most readers the inner Der wrapping is an implementation detail and not that important. But I don't feel strongly about it.
|
I pushed a commit addressing most of the review feedback that can be squash merged. |
Ah, this repo only allows the merge queue. I'll squash it manually. |
For Der and AlgorithmIdentifier, emit hex bytes rather than decimal digits separated by commas. The decimal output is impossible to read and challenging to convert into a better format. For example: [ ... 105, 103, 105, 67, 101, 114, 116, 44, 32, 73, ... ] Also, add a Debug bound on SignatureVerificationAlgorithm. This helps upstream implement Debug in a way that can list out which SignatureVerificationAlgorithms are in use.
|
Thanks for picking up the review changes, @cpu! |
For Der and AlgorithmIdentifier, emit hex bytes rather than decimal digits separated by commas. The decimal output is impossible to read and challenging to convert into a better format. For example:
[ ... 105, 103, 105, 67, 101, 114, 116, 44, 32, 73, ... ]
Also, add a Debug bound on SignatureVerificationAlgorithm. This helps upstream implement Debug in a way that can list out which SignatureVerificationAlgorithms are in use.
This copies a technique we added to rustls in rustls/rustls#1040.