Implement Debug for most types #30
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lylythechosenone
requested changes
Jun 8, 2024
src/file.rs
Outdated
Comment on lines
82
to
91
| impl Debug for MediaType { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| match self { | ||
| &Self::GENERIC => write!(f, "MediaType::GENERIC"), | ||
| &Self::OPTICAL => write!(f, "MediaType::OPTICAL"), | ||
| &Self::TFTP => write!(f, "MediaType::TFTP"), | ||
| _ => write!(f, "MediaType({})", self.0), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe use the derive(Debug) style—exclude the type name, only include the value name.
src/file.rs
Outdated
|
|
||
| match core::str::from_utf8(self.path()) { | ||
| Ok(path) => x.field("path", &path), | ||
| Err(err) => x.field("path", &err), |
There was a problem hiding this comment.
Just print bytes if the utf8 conversion fails fails.
src/file.rs
Outdated
Comment on lines
218
to
221
| x.field("tftp_ip", &self.tftp_ip()); | ||
| x.field("tftp_port", &self.tftp_port()); | ||
| #[cfg(feature = "ipaddr")] | ||
| x.field("tftp_addr", &self.tftp_addr()); |
There was a problem hiding this comment.
Instead of having a separate tftp_addr field, you could just print tftp_addr() if the feature is enabled.
src/framebuffer.rs
Outdated
Comment on lines
48
to
55
| impl Debug for MemoryModel { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| match self { | ||
| &Self::RGB => write!(f, "MemoryModel::RGB"), | ||
| _ => write!(f, "MemoryModel({})", self.0), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
See my comment on MediaType.
src/framebuffer.rs
Outdated
Comment on lines
209
to
212
| x.field("edid", &format_args!("<{} bytes>", self.edid().len())); | ||
| if let Some(modes) = self.modes() { | ||
| x.field("modes", &format_args!("<{} modes>", modes.len())); | ||
| } |
There was a problem hiding this comment.
Maybe print the full array if f.alternate() is specified ({:#?}).
src/memory_map.rs
Outdated
Comment on lines
36
to
50
| impl Debug for EntryType { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| match self { | ||
| &Self::USABLE => write!(f, "EntryType::USABLE"), | ||
| &Self::RESERVED => write!(f, "EntryType::RESERVED"), | ||
| &Self::ACPI_RECLAIMABLE => write!(f, "EntryType::ACPI_RECLAIMABLE"), | ||
| &Self::ACPI_NVS => write!(f, "EntryType::ACPI_NVS"), | ||
| &Self::BAD_MEMORY => write!(f, "EntryType::BAD_MEMORY"), | ||
| &Self::BOOTLOADER_RECLAIMABLE => write!(f, "EntryType::BOOTLOADER_RECLAIMABLE"), | ||
| &Self::KERNEL_AND_MODULES => write!(f, "EntryType::KERNEL_AND_MODULES"), | ||
| &Self::FRAMEBUFFER => write!(f, "EntryType::FRAMEBUFFER"), | ||
| _ => write!(f, "EntryType({})", self.0), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
See comment on MediaType.
src/paging.rs
Outdated
Comment on lines
41
to
59
|
|
||
| impl Debug for Mode { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] | ||
| match self { | ||
| &Self::FOUR_LEVEL => write!(f, "Mode::FOUR_LEVEL"), | ||
| &Self::FIVE_LEVEL => write!(f, "Mode::FIVE_LEVEL"), | ||
| _ => write!(f, "Mode({})", self.0), | ||
| } | ||
|
|
||
| #[cfg(target_arch = "riscv64")] | ||
| match self { | ||
| &Self::SV39 => write!(f, "Mode::SV39"), | ||
| &Self::SV48 => write!(f, "Mode::SV48"), | ||
| &Self::SV57 => write!(f, "Mode::SV57"), | ||
| _ => write!(f, "Mode({})", self.0), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
See comment on MediaType.
src/smp.rs
Outdated
Comment on lines
24
to
30
| impl Debug for GotoAddress { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| f.debug_tuple("GotoAddress") | ||
| .field(&self.inner.as_ptr()) | ||
| .finish() | ||
| } | ||
| } |
There was a problem hiding this comment.
Have this be an opaque newtype.
|
Also, you'll have to rebase from #32. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've wrote an implementation of Debug for most types, but I can't figure out how to do
FrameBufferResponse, since it gives an iterator and not a slice.Also should it print out all the framebuffer modes? There can be quite a lot of them (65 in my qemu configuration).