Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

snapbox uses green for removed and red for added lines #228

Closed
not-my-profile opened this issue Aug 18, 2023 · 3 comments · Fixed by #234
Closed

snapbox uses green for removed and red for added lines #228

not-my-profile opened this issue Aug 18, 2023 · 3 comments · Fixed by #234
Labels
A-snapbox Area: snapbox package A-trycmd Area: trycmd package question Uncertainty is involved

Comments

@not-my-profile
Copy link

#[test]
fn snapbox() {
    snapbox::assert_eq("foo", "bar");
}

screenshot of output

@epage epage added question Uncertainty is involved A-snapbox Area: snapbox package A-trycmd Area: trycmd package labels Aug 18, 2023
@epage
Copy link
Contributor

epage commented Aug 18, 2023

I'm always mixed on which way to go for this.

Generally, a diff will show the removed text as red and the added text as green.

Here, the "good" state is the base of the diff and you generally represent "good" with "green".

@not-my-profile
Copy link
Author

I appreciate that there is reasoning behind this decisions however I still consider the displaying of a diff with inversed colors to be incredibly confusing. Especially because both pretty_assertions and similar-asserts just use the regular diff colors.

When looking at a diff from an assert_eq I don't want to be forced to think to myself: "Does this assert_eq come from snapbox?" before I'm able to interpret the colors.

@marcospb19
Copy link

I agree that diff colors should be prioritized over good/bar colors for intuitiveness. However, that's probably a personal thing.

epage added a commit to epage/snapbox that referenced this issue Oct 2, 2023
Been thinking on this further and aligning with the common case is
probably more important than the possibility of what is or isn't
"right".

Fixes assert-rs#228
@epage epage closed this as completed in #234 Oct 2, 2023
indygreg pushed a commit to indygreg/trycmd that referenced this issue Nov 6, 2023
Been thinking on this further and aligning with the common case is
probably more important than the possibility of what is or isn't
"right".

Fixes assert-rs#228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-snapbox Area: snapbox package A-trycmd Area: trycmd package question Uncertainty is involved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants