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

Bug with newlines in getStructuredPatch with { newlineIsToken: true } #235

Closed
Dzenly opened this issue Nov 11, 2018 · 2 comments
Closed

Comments

@Dzenly
Copy link

Dzenly commented Nov 11, 2018

Hi!

newlineIsToken is not documented for getStructuredPatch, so may be we need throw some exception for such case.

oldFile:

a


b

newFile:

a

c
d


b

I.e. added c\nd\n\n

The result is:

[
  {
    "oldStart": 3,
    "oldLines": 0,
    "newStart": 3,
    "newLines": 1,
    "lines": [
      "+c"
    ]
  },
  {
    "oldStart": 4,
    "oldLines": 0,
    "newStart": 5,
    "newLines": 3,
    "lines": [
      "+d",
      "+",
      "+"
    ]
  }
]

So there is an error either in the first chunk, because newline is lost,
or in the second chunk, because there is wrong redundant newline.
@Dzenly
Copy link
Author

Dzenly commented Nov 11, 2018

The same for createPath with newlineIsToken: true

Index: someFIleName
===================================================================
--- someFIleName	oldHeader
+++ someFIleName	newHeader
@@ -3,0 +3,1 @@
+c
@@ -4,0 +5,3 @@
+d
+
+

And the same bug is for diffLines with newlineIsToken: true:

[ { count: 3, value: 'a\n\n' },
  { count: 1, added: true, removed: undefined, value: 'c' },
  { count: 1, value: '\n' },
  { count: 4, added: true, removed: undefined, value: 'd\n\n\n' }, // No, one newline was here, so there should be only 2 newlines.
  { count: 2, value: 'b\n' } ]

@ExplodingCabbage
Copy link
Collaborator

Not sure what change fixed it or when it happened, but the diffLines bug seems to be fixed on master?

> oldFile
'a\n\n\nb\n\n'
> newFile
'a\n\nc\nd\n\n\nb\n'
> diff.diffLines(oldFile, newFile).filter(obj => !obj.removed).map(obj => obj.value).join('')
'a\n\nc\nd\n\n\nb\n'
> diff.diffLines(oldFile, newFile, {newlineIsToken: true}).filter(obj => !obj.removed).map(obj => obj.value).join('')
'a\n\nc\nd\n\n\nb\n'
> diff.diffLines(oldFile, newFile)
[
  { count: 2, added: false, removed: false, value: 'a\n\n' },
  { count: 2, added: true, removed: false, value: 'c\nd\n' },
  { count: 1, added: false, removed: false, value: '\n' },
  { count: 1, added: false, removed: true, value: 'b\n' },
  { count: 1, added: false, removed: false, value: '\n' },
  { count: 1, added: true, removed: false, value: 'b\n' }
]
> diff.diffLines(oldFile, newFile, {newlineIsToken: true})
[
  { count: 3, added: false, removed: false, value: 'a\n\n' },
  { count: 1, added: true, removed: false, value: 'c' },
  { count: 1, added: false, removed: false, value: '\n' },
  { count: 1, added: false, removed: true, value: 'b' },
  { count: 1, added: true, removed: false, value: 'd' },
  { count: 2, added: false, removed: false, value: '\n\n' },
  { count: 3, added: true, removed: false, value: '\nb\n' }
]

Can't find a bug in the patch methods either:

> diff.applyPatch(oldFile, diff.structuredPatch("old", "new", oldFile, newFile))
'a\n\nc\nd\n\n\nb\n'
> newFile
'a\n\nc\nd\n\n\nb\n'
> diff.applyPatch(oldFile, diff.structuredPatch("old", "new", oldFile, newFile, {newlineIsToken: true}))
'a\n\nc\nd\n\n\nb\n'
> console.log(JSON.stringify(diff.structuredPatch("old", "new", oldFile, newFile), null, 2))
{
  "oldFileName": "old",
  "newFileName": "new",
  "hunks": [
    {
      "oldStart": 1,
      "oldLines": 5,
      "newStart": 1,
      "newLines": 7,
      "lines": [
        " a",
        " ",
        "+c",
        "+d",
        " ",
        "-b",
        " ",
        "+b"
      ]
    }
  ]
}

Am gonna assume whatever was wrong here is fixed and close.

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

No branches or pull requests

2 participants