arm: use docsplay::Display for ArmError #2702
Conversation
bugadani
added
the
changelog:skip
probe-rs/src/architecture/arm/mod.rs
Outdated
| ReAttachRequired, | ||
|
|
||
| /// An operation was not performed because the required permissions were not given. | ||
| /// An operation could not be performed because it lacked the permission to do so: {0}. |
There was a problem hiding this comment.
This might be a bit verbose to display as an error. You may add #[ignore_extra_doc_attributes] so that only the first line will be part of the error message.
There was a problem hiding this comment.
I thought it’d do that by default.
There was a problem hiding this comment.
displaydoc would IIRC simply fail in this situation, docsplay can handle all the docs you give it. Since the library is not only meant for error messages, it defaults to displaying everything.
There was a problem hiding this comment.
BTW, I never used it before, I just saw it used in another module and figured this was a good way to reduce the amount of noise in the code.
There was a problem hiding this comment.
Yeah, it's my fork of displaydoc because I had a few issues with it, but it's not perfect. The formatting placeholders can easily introduce inelegant noise into the documentation, and automatically trying to fix that is pretty hard.
ithinuel
force-pushed
the
use-docsplay
branch
3 times, most recently
from
b3a7762 to
e7a9030
Compare
| @@ -0,0 +1 @@ | |||
| Changed ArmError’s documentation to use docsplay::Display and avoid repetition | |||
There was a problem hiding this comment.
So, do you really want to keep this? :)
There was a problem hiding this comment.
I don’t mind either way. It was only to make GH checks happy :)
There was a problem hiding this comment.
The changelog check is not important, there's a label to pacify it :)
probe-rs/src/architecture/arm/mod.rs
Outdated
| pub enum ArmError { | ||
| /// The operation requires a specific architecture. | ||
| #[error("The operation requires one of the following architectures: {0:?}.")] | ||
| /// The operation requires one of the following architectures: {0:?}. | ||
| ArchitectureRequired(&'static [&'static str]), | ||
|
|
||
| /// A timeout occurred during an operation |
There was a problem hiding this comment.
| /// A timeout occurred during an operation | |
| /// A timeout occurred during an operation. |
There was a problem hiding this comment.
Should I also add a . after the other {0} ?
There was a problem hiding this comment.
no, I think that would introduce double dots in some cases.
probe-rs/src/architecture/arm/mod.rs
Outdated
| RegisterParse(#[from] RegisterParseError), | ||
|
|
||
| /// Error reading ROM table. | ||
| #[error("Error reading ROM table.")] | ||
| RomTable(#[source] RomTableError), | ||
|
|
||
| /// Failed to erase chip |
There was a problem hiding this comment.
| /// Failed to erase chip | |
| /// Failed to erase chip. |
This avoids duplication of the documentation
Follows: