Fix panic with custom Debug impl returning an empty string #26
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
The panic was 'attempt to subtract with overflow' because the first difference is addition. PrettyString is from idubrov: #24 (comment)
colin-kiegel
requested changes
Feb 14, 2019
| @@ -36,7 +36,7 @@ pub fn format_changeset(f: &mut fmt::Formatter, changeset: &Changeset) -> fmt::R | |||
| } | |||
| } | |||
| Difference::Add(ref added) => { | |||
| match diffs.get(i - 1) { | |||
| match diffs.get(i.wrapping_sub(1)) { | |||
There was a problem hiding this comment.
I'd prefer not to rely on the last index to be None, but instead handle i=0 explicitly, e.g.
let prev = i.checked_sub(1).and_then(|x| diffs.get(x));
match prev {
|
Thanks for taking a look! Please feel free to incorporate the suggested adjustment yourself (I don't care about attribution in the commit history, if that's why you asked, and can't make the change easily). |
|
merged manually :-) |
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 tried to use the idea posted in #24 by @idubrov.
When running:
I got a panic 'attempt to subtract with overflow'. This PR fixes it.