Improve debug wording #311
Conversation
src/least_satisfying.rs
Outdated
| Self::Yes => write!(f, "Does not reproduce the regression"), | ||
| Self::No => write!(f, "Reproduces the regression"), |
There was a problem hiding this comment.
I think this might be backwards.
Yes means it reproduces the regression (rustc failed to compile).
No means it did not reproduce (rustc compiled successfully).
However, the terms "regress" and "not regress" may not be completely clear. For example, you might be looking for when something was fixed, such as when using --regress=no-error, which has an opposite meaning ("no" means "fails to compile", "yes" means "compiles successfully").
I would be fine with merging this with the text reversed, since it is a little clearer than "yes" and "no", but I suspect there is still room for confusion here. I think longer term it would be better to have customizable terms, and then things like --regress=no-error could change them to "compiles successfully" and "failed to compile", and offer the ability to customize the message like git bisect does (maybe --term-old=good --term-new=bad).
There was a problem hiding this comment.
thanks @ehuss exactly the feedback I needed
Ok I've fixed the "logical" error.
Now, for a better wording. The Yes/No in the end means whether the toolchain at hand did produce the expected result using the sample code provided, correct? "Yes" (it did) or "No" (it didn't).
I couldn't think of a single sentence to squeeze all "success" cases and all "failure" cases into a single sentence. I agree that the message should be contextual to the --regress parameter used.
Unless we use some vague wording like "requested condition matched" or "not matched".
wdyt ?
There was a problem hiding this comment.
Unless we use some vague wording like "requested condition matched" or "not matched".
That actually sounds a little better to me. I do worry that specifically saying Reproduces the regression could be confusing if you are searching for when the regression was fixed (since it will be backwards).
I opened #316 for being able to customize it. I can work on that sometime later unless you are interested.
apiraino
force-pushed
the
improve-wording
branch
from
0a54a66
to
917d8d8
Compare
A user recently mentioned that the wording of cargo bisect during the bisection is a bit unclear (https://togithub.com/rust-lang/rust/issues/119822#issuecomment-1929153702), namely what do the messages after every test mean.
With this patch I'm trying to make it clearer (I hope I got the logic right).
thanks for checking this out!
r? @ehuss