Make Report::report
print the "Error: " prefix
#453
Merged
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.
Fixes #452
My reasoning to address the concern mentioned in the issue:
It seems to me that is sufficiently ensured:
When
#[snafu::report]
transformsmain
, or a#[test]
function, to return-> Result<(), Report<E>>
, to support older versions of Rust (i.e. when the package featurerust_1_61
is not enabled), or when that same return type was chosen explicitly (i.e. without using the attribute-macro), it is insteadResult as Termination
that is used. That different implementation, with theError:
prefix printed by it instead, uses theDebug
formatting ofReport
(which is the same as itsDisplay
impl), for which SNAFU already does not add any prefix. So, it's impossible for an extra prefix to be printed due to my change, for all preexisting and future uses of<Result<T, Report<E>> as Termination>::report
.Only
<Report as Termination>::report
is changed, and so the only way extraError:
(or the like) could be printed is if a user chooses to add their own such extra print before that function is called.Given the narrow purpose of that function, it seems very unlikely that anyone is calling it other than implicitly after
main
, or a#[test]
fn, returns.<T as Termination>::report
in a generic context anywhere (e.g. in the body of some test of that functionality), it seems likely they'd already be prepared for it to print the prefix, because thestd
library already does that forResult
, and so they wouldn't have added their own printing of a prefix.<Report as Termination>::report
, but if they are then they shouldn't have relied on it to not print the prefix, because SNAFU's documentation already implies that it will print the prefix.If any user has been printing their own prefix before
main
, or a#[test]
fn, returns, then they should already be expecting an extra prefix, because without featurerust_1_61
(e.g. for older versions of Rust) theError:
prefix was already being printed by thestd
lib's<Result<(), Report<E>> as Termination>::report
, and so such users should not have expected it to not continue doing that with newer Rust versions when<Report as Termination>::report
is used in what was intended to be an equivalent internal-detail change.SNAFU's documentation says:
I added a test to ensure that
<Report as Display>::fmt
continues to not add an "error" prefix, to ensure that point 1 continues to be upheld.I tried to think if any other tests should be added for any other aspects of this, but couldn't think of anything, because it's not possible to redirect the
stderr
output of a#[test]
function to check it, nor did I notice any preexisting facility for checking the output of a test program (in contrast to the preexistingcompatibility-tests/compile-fail/
), as would be needed to test that the prefix is now being printed.