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

Nicer formatting for multiline strings #24

Closed
Nemo157 opened this issue Jun 10, 2018 · 14 comments · Fixed by #88
Closed

Nicer formatting for multiline strings #24

Nemo157 opened this issue Jun 10, 2018 · 14 comments · Fixed by #88

Comments

@Nemo157
Copy link

Nemo157 commented Jun 10, 2018

While the current output is miles ahead of the default assert_eq! for finding the errors in multiline strings, it would be amazing if multiline strings could actually be displayed as multiline. Another extension to this would be to use raw string literals when the string contains embedded " characters

Current output (0.5.1):

screen shot 2018-06-10 at 12 42 46 pm

A quick sketch of what multiline and raw string literals could look like:

screen shot 2018-06-10 at 12 51 25 pm

I'm not sure if there are cases where this would be sub-optimal, hopefully it's possible to use a simple heuristic for detecting when to enable output like this that can be used in all cases.

@Nemo157
Copy link
Author

Nemo157 commented Jun 10, 2018

Pushed a super-quick implementation of this: Nemo157@9332632

Probably needs a bit better heuristics, but it at least works for my usecase:

screen shot 2018-06-10 at 1 48 43 pm

EDIT: Let me know if you're interested in the feature and I'll try to polish it and submit a PR.

@idubrov
Copy link

idubrov commented Jan 12, 2019

Here is a non-heuristic variant we use:

/// Wrapper around string slice that makes debug output `{:?}` to print string same way as `{}`.
/// Used in different `assert*!` macros in combination with `pretty_assertions` crate to make
/// test failures to show nice diffs.
#[derive(PartialEq, Eq)]
#[doc(hidden)]
pub struct PrettyString<'a>(pub &'a str);

/// Make diff to display string as multi-line string
impl<'a> fmt::Debug for PrettyString<'a> {
  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
    f.write_str(self.0)
  }
}

but you have to wrap strings manually: assert_eq!(PrettyString("hello"), PrettyString("bye")).

colin-kiegel pushed a commit that referenced this issue Feb 15, 2019
The panic was 'attempt to subtract with overflow' because
the first difference is addition.

PrettyString is from idubrov:
#24 (comment)
@colin-kiegel
Copy link
Collaborator

@Nemo157 Yes, I would be interested in the feature - but maybe behind a feature flag (we can discuss, what the default should be). Are you still interested in contributing to this feature?

Sorry for replying so late..

@colin-kiegel colin-kiegel pinned this issue Feb 21, 2019
@rfdonnelly
Copy link

I've extended @idubrov solution to automate the wrapping of strings with a PrettyString by adding a local version of the assert_eq!() macro that wraps the strings and calls pretty_assertions::assert_eq!().

/// Wrapper around string slice that makes debug output `{:?}` to print string same way as `{}`.
/// Used in different `assert*!` macros in combination with `pretty_assertions` crate to make
/// test failures to show nice diffs.
#[derive(PartialEq, Eq)]
#[doc(hidden)]
pub struct PrettyString<'a>(pub &'a str);

/// Make diff to display string as multi-line string
impl<'a> fmt::Debug for PrettyString<'a> {
  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
    f.write_str(self.0)
  }
}

macro_rules! assert_eq {
    ($left:expr, $right:expr) => {
        pretty_assertions::assert_eq!(PrettyString($left), PrettyString($right));
    }
}

Now you can just call assert_eq!() as normal:

assert_eq!(left, right);

@Herschel
Copy link

This would be a very helpful feature. I'm currently using the suggestion above by @rfdonnelly, which works great!

@lig
Copy link

lig commented Jul 29, 2020

@colin-kiegel it looks like adding something special to assert_eq! call to use diff by Display instead of diff by Debug is not a problem for users. Maybe, it would be enough to add an optional third argument to the signature, i.e. passing the method to use explicitly.

Examples

// uses default behavior, could include a heuristics of some kind to guess the method
assert_eq!(a, b)

// uses Debug and fails if it's impossible
assert_eq!(a, b, pretty_assertions::Method::Debug)

// uses Display and fails if it's impossible
assert_eq!(a, b, pretty_assertions::Method::Display)

@Wilfred
Copy link

Wilfred commented Sep 30, 2020

One workaround I've found helpful is converting my strings to a vec of lines, and then using assert_eq with that.

@dralley
Copy link

dralley commented Mar 5, 2021

I'm trying to compare XML and this would be a lifesaver. It looks like there's some activity in this repo again, is this being looked at?

@colin-kiegel
Copy link
Collaborator

colin-kiegel commented Mar 12, 2021

I can see there is quite some interest in a different representation of multiline string diffs and I would appreciate it behind a feature flag or something similar at first - to be on the safe side.

The goal of pretty_assertions is to be a drop-in-replacement for assert_eq! with different formatting. This is why I try to keep it similar at its roots, IMO this implies:

  • pretty_assertions::assert_eq! should have the same signature as the original - you should be able to switch back to the original at any time without much churn
  • as the original assert_eq! we should compare the fmt::Debug output instead of fmt::Display - at least by default - to guarantee that we are giving the same output just with different formatting

With these limitations I am open to discuss different formatting approaches.

@tommilligan What are your thoughts (and conditions) on this as new Co-Maintainer? :-)

@tommilligan
Copy link
Collaborator

I've been thinking about this in the background, as it seems to be a common theme around many issues/PRs in this repo (there are currently 3 PRs open to do with customising/tweaking the output of pretty_assertions). There's obviously interest in customising the output, but no two people agree on the same output format.

As I see it, the core functions of pretty_assertions for the user are:

  • Provide macros fitting the assert_eq and assert_ne API
  • Provide some level of diff/diffing algorithm
  • Print a diff in a pretty manner

To this end, I think we have two options:

  1. Provide tonnes of feature flags, config options etc. to make the behaviour of the library customisable
  2. Refactor the existing implementation into a library of components, enabling users to build their own solutions on top of it

I would vastly prefer option 2., which I think:

  • Adheres to the existing library concerns
  • Is cleaner in design
  • Will be more maintainable over the long term

With this design, a PR like #33 could instead be written as a new crate/module, which pulls in functionality from the pretty_assertions library for diffing and printing, but provides it's own macro implementation.


In my eyes, good first steps in this design path would be:

  • Pulling apart the diffing/printing logic we currently use, which is tightly linked together.
  • Providing a helper to construct custom macro implementations, given a differ and printer. I have no idea if such a thing is even possible, so this would need research.

@colin-kiegel
Copy link
Collaborator

colin-kiegel commented Apr 17, 2021

@tommilligan I like your line of thinking, to provide tutorials and basic building blocks for custom implementations (=library-approach). This sounds like a good tradeoff with high flexibility and low maintenance burden.

We can then observe whether specific extensions become particularly popular - and still decide whether we want to include them later.

Alternatively to the "library"-metaphor, we could also investigate a "framework"-approach - i.e. maybe we could allow the consumer to define callback-functions and thereby overriding the default behaviour. It would be interesting to investigate the pros and cons of both approaches - both seem very promising to me. :-)

@categulario
Copy link

Hi everyone,

I was just looking for a way to display a nice diff in my asserts and found this project and was very excited. Then I looked at the default string diff and was very disappointed and eventually ran into this thread. My two cents: I bet a nice multi-line string diff is the expected default (it was for me). And while the solution mentioned by @rfdonnelly works just fine it is a problem having to first find this thread, then copy the code, then see the result you're expecting. I don't think all of the people that need this feature get this far.

That being said, If I can contribute to make it happen (behind a feature flag or something (which I would recommend be visibly documented in the first paragraphs of the readme)) I'll happily do it.

Thank you for this amazing project.

x3ro added a commit to x3ro/rust-pretty-assertions that referenced this issue Jan 31, 2022
As discussed in rust-pretty-assertions#24, pretty_assertion's `assert_eq` will escape line
breaks in multi-line strings, making diffs for them much harder to
read.

This commit introduces another variant, `assert_str_eq`, and the
associated `StrComparison`, which compares the raw strings, where
the newlines are not escaped.

Fixes rust-pretty-assertions#24
@x3ro
Copy link
Contributor

x3ro commented Jan 31, 2022

First of all, I want to say thanks to everyone who has contributed to pretty_assertions in the past, I think it's a great library.

Since this discussion has stagnated, I just wanted to throw a proposal for a solution out there (see #88). It goes the "easy" route by introducing a new macro variant for strings, assert_str_eq!. This variant only works for strings, and does not escape line breaks in multi-line strings.

Of course this is not a perfect solution, because the user has to know that this additional variant exists. I think however, that it's preferable to library users having to first find out that this is expected behavior (e.g. through this issue), and then having to implement a custom solution for each project they use the crate in.

I've read @tommilligan's earlier statement of not wanting to bloat the library with lots of variants / configuration options, so I realize that this may not get merged. While I agree that a library such as this one should not try to provide everything for everyone, I think that diff'ing a plain string in a readable way is relatively basic functionality which, in my opinion, this library should provide in some way.

x3ro added a commit to x3ro/rust-pretty-assertions that referenced this issue Feb 1, 2022
As discussed in rust-pretty-assertions#24, pretty_assertion's `assert_eq` will escape line
breaks in multi-line strings, making diffs for them much harder to
read.

This commit introduces another variant, `assert_str_eq`, and the
associated `StrComparison`, which compares the raw strings, where
the newlines are not escaped.

Fixes rust-pretty-assertions#24

Co-authored-by: Tom Milligan <tom@reinfer.io>
x3ro added a commit to x3ro/rust-pretty-assertions that referenced this issue Feb 1, 2022
As discussed in rust-pretty-assertions#24, pretty_assertion's `assert_eq` will escape line
breaks in multi-line strings, making diffs for them much harder to
read.

This commit introduces another variant, `assert_str_eq`, and the
associated `StrComparison`, which compares the raw strings, where
the newlines are not escaped.

Fixes rust-pretty-assertions#24

Co-authored-by: Tom Milligan <tom@reinfer.io>
@tommilligan
Copy link
Collaborator

assert_str_eq has been released in v1.1.0, which I believe should broadly cover the issues discussed here - thanks to @x3ro for implementing this and pushing it forward!

Please take a look and see if this covers your use cases. If not, feel free to open a new issue for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.