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

Add multiline string diffs #6

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add multiline string diffs #6

wants to merge 5 commits into from

Conversation

1Jajen1
Copy link
Member

@1Jajen1 1Jajen1 commented Feb 19, 2020

This lays out multiline strings line by line and compares them, producing a much nicer diff on failure.

A test like this:

"Hello\nWorld\n?".eqv("Hello\nWelt\n?!")

Produces:

- Hello
 World
 ?
+ Hello
 Welt
 ?!

But now with this pr it produces:

  1. Hello
- 2. World
+ 2. Welt
- 3. ?
+ 3. ?!

This is only enabled for strings that contain \n. All others will still render on top of each other without line numbers.

This lays out multiline strings line by line and compares them, producing a much nicer diff on failure.
@1Jajen1
Copy link
Member Author

1Jajen1 commented Feb 19, 2020

Thanks @pakoito, now to figure out why the build did not run...

@1Jajen1
Copy link
Member Author

1Jajen1 commented Feb 19, 2020

Btw the description is not 100% accurate:
Because the algorithm for diffing is a ses algorithm the diff will not be line by line.

This examples shows this better:

"Hello\nWOrld\n\n.".eqv("Hello\nWorld\n!\n.")
  1. Hello
- 2. WOrld
- 3.
+ 2. World
+ 3. !
  4. .

This is very useful if long multiline texts change by removing or inserting a newline in the middle, however this sucks for short multiline text where I'd prefer:

  1. Hello
- 2. WOrld
+ 2. World
- 3.
+ 3. !
  4. .

Both things are super easy to implement and I don't really have a strong opinion on either. (The current one is just nice because it reuses list diffing).

Thoughts?

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

Successfully merging this pull request may close these issues.

3 participants