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

applyPatch affecting wrong line number with with new lines #159

Closed
Luiz-N opened this issue Dec 13, 2016 · 4 comments
Closed

applyPatch affecting wrong line number with with new lines #159

Luiz-N opened this issue Dec 13, 2016 · 4 comments

Comments

@Luiz-N
Copy link

Luiz-N commented Dec 13, 2016

@kpdecker your library is great so far. I think I might have found an edge case unless i'm missing something?

oldString = 'test\ntest2\n \ntest3\n';
"test
test2
 
test3
"
newString = 'test\ntest2\ntest3\n';
"test
test2
test3
"

Now I create a patch

d = JsDiff.createPatch('file/path.js', oldString, newString, '', '', { context: 0 });

This patch/diff looks correct

"Index: file/path.js
===================================================================
--- file/path.js	
+++ file/path.js	
@@ -3,1 +3,0 @@
- 

However when I apply the patch....

result = JsDiff.applyPatch(oldString, d);
"test
test2
 
"

We can see it seems to be affecting the wrong line number. Is it trimming somewhere on the oldString where it shouldn't be?

@Luiz-N
Copy link
Author

Luiz-N commented Dec 14, 2016

UPDATE:

I've narrowed the scope of the bug down to when context is set to 0. So by setting it to 1 or higher it works correctly:

d = JsDiff.createPatch('file/path.js', oldString, newString, '', '', { context: 1 });

result = JsDiff.applyPatch(oldString, d);
"test
test2
test3
"

@ibc
Copy link

ibc commented Mar 14, 2017

Good catch. Same happens here.

@bp-dev
Copy link
Contributor

bp-dev commented Jan 9, 2018

Also having problems with empty lines in 3 cases.

  1. a patch starting with a empty line of context such as
@ -92,6 +92,19 @@

         <div id="secondaryToolbar" class="secondaryToolbar hidden doorHangerRight">
           <div id="secondaryToolbarButtonContainer">
+            <button id="secondaryOnePageMode" class="secondaryToolbarButton onePageMode visibleLargeView" title="One Page Mode" tabindex="51" data-l10n-id="one_page_mode">

is not applied (and following blocks are applied in the wrong place as line numbers don't match anymore)

  1. a patch containing an empty line of context: the diff after the empty line is ignored as parsing stops
@@ -26,19 +26,8 @@
     <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
     <meta name="google" content="notranslate">
     <meta http-equiv="X-UA-Compatible" content="IE=edge">
-    <title>PDF.js viewer</title>
-
-
-    <link rel="stylesheet" href="viewer.css">
-
-
-
-
-
-<!-- This snippet is used in production (included from viewer.html) -->
-<link rel="resource" type="application/l10n" href="locale/locale.properties">
-<script src="pdf.viewer.js"></script>

+<!-- == INSERTS == -->

   </head>

  1. a patch with a trailing empty line at the end (added by a text editor or whatever) runs endlessly after simple correction of the 2 above and requires to be handled specifically.

Pull request #212 fixes those cases (as far as I can test, comparing on a limited number of real-life samples with the behaviour of the linux patch command, which I would consider a good reference)

@Luiz-N
Copy link
Author

Luiz-N commented Jul 23, 2018

I think this got fixed with #212

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

3 participants