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

Threat each newline as separate word in diffWordsWithSpace #217

Merged
merged 1 commit into from
Aug 16, 2020

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Mar 10, 2018

This fixes #180 & #211

@Mingun
Copy link
Contributor Author

Mingun commented Mar 20, 2018

@kpdecker any feedback?

@Mingun
Copy link
Contributor Author

Mingun commented Mar 19, 2020

@kpdecker, can we make progress with this? Better late than never

@kpdecker kpdecker merged commit 6464b29 into kpdecker:master Aug 16, 2020
@kpdecker
Copy link
Owner

Sorry for the delay on this, Have very few cycles for this project these days.

@Mingun
Copy link
Contributor Author

Mingun commented Aug 16, 2020

Nice to see that you don't forgot the project

@Mingun Mingun deleted the fix-newline-word branch August 16, 2020 21:01
tobiasso85 added a commit to SAP/ui5-migration that referenced this pull request Apr 1, 2022
tobiasso85 added a commit to SAP/ui5-migration that referenced this pull request Apr 1, 2022
* Bump diff from 4.0.2 to 5.0.0

Bumps [diff](https://github.com/kpdecker/jsdiff) from 4.0.2 to 5.0.0.
- [Release notes](https://github.com/kpdecker/jsdiff/releases)
- [Changelog](https://github.com/kpdecker/jsdiff/blob/master/release-notes.md)
- [Commits](kpdecker/jsdiff@v4.0.2...v5.0.0)

Signed-off-by: dependabot[bot] <[email protected]>

* fixed tests, the changes to the expected test files' content is minimal
and there is no best solution.

Breaking change is:
kpdecker/jsdiff#217

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tobias Sorn <[email protected]>
@ExplodingCabbage
Copy link
Collaborator

IMO the behaviour this produces for diffWords is broken as a result of the extra tokens. Here's a diff using the code before this change...

> wd.diffWords("foo bar", "foo\n\nbar")
[ { value: 'foo\n\nbar', count: 3 } ]

... and here's one with the code after this change:

> wd.diffWords("foo bar", "foo\n\nbar")
[
  { count: 2, value: 'foo\n' },
  { count: 1, added: true, removed: undefined, value: '\n' },
  { count: 1, value: 'bar' }
]

We're now only sort-of-ignoring whitespace in diffWords.

(In fairness, we were already doing a bad job of ignoring whitespace in diffWords; if you changed the whitespace around some punctuation it would be treated as a change, before.)

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.

diffWordsWithSpace - a char-by-char diff for spaces?
3 participants