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

Reverse multi-line diffs to standard order #1647

Closed
wants to merge 2 commits into from

Conversation

nylen
Copy link
Contributor

@nylen nylen commented Apr 8, 2015

Display multi-line diffs in the same standard order as diff, git, etc:

- actual / removed
+ expected / added

The current order is unintuitive which makes it harder to read at a glance:

+ expected / added
- actual / removed

Pending resolution of kpdecker/jsdiff#14 this works by reversing the added and removed strings passed to diff.createPatch, and then reversing the + and - status of each change.

@boneskull
Copy link
Contributor

@nylen I don't understand what this has to do with jsdiff?

@boneskull
Copy link
Contributor

@nylen does this depend on #1648 at all?

@nylen
Copy link
Contributor Author

nylen commented Apr 12, 2015

This is really a jsdiff issue but we can and should work around it here, since there doesn't seem to be a clean way to tell jsdiff to do this.

This and #1648 are both usability improvements to diffs, but they are independent changes.

@nylen
Copy link
Contributor Author

nylen commented Apr 15, 2015

@boneskull this PR and #1648 are big wins for diff usability, can they be merged please?

before:
screen shot 2015-04-15 at 2 12 17 pm

after:
screen shot 2015-04-15 at 2 13 29 pm

@nylen
Copy link
Contributor Author

nylen commented Apr 24, 2015

@a8m @boneskull @danielstjules @dasilvacontin @jbnicolai @pkozlowski-opensource @travisjeffery review please? I'd really like to get this (or something like it) merged. Same goes for #1648.

@nylen nylen mentioned this pull request May 5, 2015
@nylen
Copy link
Contributor Author

nylen commented May 7, 2015

ONE MONTH LATER

Now that kpdecker/jsdiff#14 is fixed, this PR is obsolete. The correct fix is to upgrade to the newest version of jsdiff.

@nylen nylen closed this May 7, 2015
@danielstjules
Copy link
Contributor

Awesome! And sorry for the delay! Glad it was fixed upstream, too, since I use it on other projects :)

@nylen nylen mentioned this pull request May 13, 2015
@nylen nylen deleted the reverse-diffs branch June 25, 2018 02:31
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