Re: [Monotone-devel] BUG: "mtn diff" vs. "diff" with respect to not existing trailing newline on last line

[Ralf S. Engelschall]

On Wed, Feb 27, 2008, Ralf S. Engelschall wrote:

> I got catched the second time by this bug and now discovered the root of
> the problem and hence finally can show a reproducer:
> [...]
> The problem is that the generated diff for test2.txt is not correct.
> I've now just added the line with "baz", but actually also changed the
> line with "quux": I added the terminating newline!
>
> As a result patch(1) is not able to apply the generated patch, of
> course! The correct patch as produced by diff(1) is:
>
> | $ diff -u3 test1.txt.orig test1.txt; diff -u3 test2.txt.orig test2.txt
> | --- test1.txt.orig      2008-02-27 19:26:07 +0100
> | +++ test1.txt   2008-02-27 19:26:26 +0100
> | @@ -1,3 +1,5 @@
> |  foo
> |  bar
> |  quux
> | +
> | +baz
> | --- test2.txt.orig      2008-02-27 19:26:07 +0100
> | +++ test2.txt   2008-02-27 19:26:28 +0100
> | @@ -1,3 +1,4 @@
> |  foo
> |  bar
> | -quux
> | \ No newline at end of file
> | +quux
> | +baz
>
> Notice the difference on the "quux" line for test2.txt and the special
> case of the "\ No newline at end of file" marker. I think Monotone has
> to do the same in order to allow its output to be correctly processed by
> the all-dancing-all-singing patch(1) command. I've still not looked at
> the Monotone code, but I guess there currently might be still no special
> processing for this special case...

Ok, found the problem. The diff implementation of Monotone just strips
away all line termination characters and hence is not aware at all of
the special case of a not newline terminated last line.

The appended patch successfully resolves the bug for me.
It does:

1. ensures that also for the special cases the order is:
   first output deleted lines, then added lines. (Don't be confused by
   the "mtn diff" output, I just swapped two if-clauses as-is, although
   the diff produces smaller hunks because the two if-clauses were such
   similar).

2. introduces a "for_diff" boolean to split_into_lines()
   which indicates that content "foo" on a last non- newline terminated
   line should be rendered as "foo[\r]\n\\ No newline at end of file".
   This special treatment is necessary as split_into_lines() is also
   used for non-diff processing.

                                       Ralf S. Engelschall
                                       address@hidden
                                       www.engelschall.com

============================================================
--- diff_patch.cc       e359a4c03c6ce548d6414dce11160fbcb6fcb831
+++ diff_patch.cc       dd0193ceccadfb2f32b3ad52b3eb6a9f06786c1a
@@ -1010,17 +1010,17 @@ void walk_hunk_consumer(vector<long, QA(
           while (idx(lines2,b) != *i)
             cons.insert_at(b++);
         }
-      if (b < lines2.size())
+      if (a < lines1.size())
         {
           cons.advance_to(a);
-          while(b < lines2.size())
-            cons.insert_at(b++);
+          while(a < lines1.size())
+            cons.delete_at(a++);
         }
-      if (a < lines1.size())
+      if (b < lines2.size())
         {
           cons.advance_to(a);
-          while(a < lines1.size())
-            cons.delete_at(a++);
+          while(b < lines2.size())
+            cons.insert_at(b++);
         }
       cons.flush_hunk(a);
     }
@@ -1349,8 +1349,8 @@ make_diff(string const & filename1,
     }

   vector<string> lines1, lines2;
-  split_into_lines(data1(), lines1);
-  split_into_lines(data2(), lines2);
+  split_into_lines(data1(), constants::default_encoding, lines1, true);
+  split_into_lines(data2(), constants::default_encoding, lines2, true);

   vector<long, QA(long)> left_interned;
   vector<long, QA(long)> right_interned;
============================================================
--- simplestring_xform.cc       5f2e4fcd288a2ec1fd59feb97ebc777b56597608
+++ simplestring_xform.cc       6219eea46b1cb345594d8f46d777083ef907cb01
@@ -53,6 +53,14 @@ void split_into_lines(string const & in,
                       string const & encoding,
                       vector<string> & out)
 {
+  return split_into_lines(in, encoding, out, false);
+}
+
+void split_into_lines(string const & in,
+                      string const & encoding,
+                      vector<string> & out,
+                      bool for_diff)
+{
   string lc_encoding = lowercase(encoding);
   out.clear();

@@ -92,8 +100,14 @@ void split_into_lines(string const & in,
             break;
           end = in.find_first_of("\r\n", begin);
         }
-      if (begin < in.size())
-        out.push_back(in.substr(begin, in.size() - begin));
+      if (begin < in.size()) {
+          string s = in.substr(begin, in.size() - begin);
+          if (for_diff) {
+            s += (in.find_first_of("\r") != string::npos ? "\r\n" : "\n");
+            s += "\\ No newline at end of file";
+          }
+          out.push_back(s);
+      }
     }
   else
     {
============================================================
--- simplestring_xform.hh       0280d49b889bc0b7b2900d5d123cc8ec95a115eb
+++ simplestring_xform.hh       daf1a9e8d735b11be144e8fa4160c1865b700761
@@ -13,6 +13,11 @@ void split_into_lines(std::string const
                       std::string const & encoding,
                       std::vector<std::string> & out);

+void split_into_lines(std::string const & in,
+                      std::string const & encoding,
+                      std::vector<std::string> & out,
+                      bool for_diff);
+
 void join_lines(std::vector<std::string> const & in,
                 std::string & out,
                 std::string const & linesep);