[Monotone-devel] [REVIEW] automate content_diff

[Thomas Keller]

Hi!

Please have a look at the attached patch. I have to excuse that there
are some unrelated changes in this patch (basically renaming of an
automate test to better fit the automate command and some formatting
fixes in monotone.texi) - this won't happen again.

If everything looks alright, I'd happily merge this into nvm soon.

Thomas.

-- 
- "I know that I don't know." (Sokrates)
Guitone, a frontend for monotone: http://guitone.berlios.de
Music lyrics and more: http://musicmademe.com

# 
# old_revision [2c7f4cc388667aa2d233aa9e0749b317ac42f699]
# 
# rename "tests/automate_automate_version"
#     to "tests/automate_interface_version"
# 
# add_dir "tests/automate_content_diff"
# 
# add_file "tests/automate_content_diff/__driver__.lua"
#  content [49eb0b60c12eefb624af0e586d6bd860ac03cfb4]
# 
# patch "automate.cc"
#  from [092f5069e1ce9988dc6260f868f3a9cd7f14e187]
#    to [a6332c5bb3ceb42b9829830ae0935794086e3f2a]
# 
# patch "cmd_diff_log.cc"
#  from [0eee6f1675c850932e53b7a0214235ae0dc65ad4]
#    to [ad597a711af534bd952b3753d0af247f9e8c1223]
# 
# patch "commands.cc"
#  from [01337415288f19b925c2097f54cb3fe05fce3ced]
#    to [a46a10ed30eab12e318db1db9b48d100f1009ce1]
# 
# patch "monotone.texi"
#  from [18658fe14fa458d5592b1810fc8b353551d14719]
#    to [7212dc67eee89e3f633aaf15f44ef3e265010a92]
# 
# patch "testsuite.lua"
#  from [67b5f8eb1b321452edafe257ac5f61e82c85c856]
#    to [ee3dadaa84390b8be6919ce41c9c5e6c4e3b4881]
# 
============================================================
--- tests/automate_content_diff/__driver__.lua  
49eb0b60c12eefb624af0e586d6bd860ac03cfb4
+++ tests/automate_content_diff/__driver__.lua  
49eb0b60c12eefb624af0e586d6bd860ac03cfb4
@@ -0,0 +1,36 @@
+
+mtn_setup()
+
+-- check output if there are no changes
+check(mtn("automate", "content_diff"), 0, true, true)
+check(fsize("stdout") == 0 and fsize("stderr") == 0)
+
+-- check non-existing path
+check(mtn("automate", "content_diff", "non_existing"), 1, true, true)
+
+
+-- check existing path against current workspace
+addfile("existing", "foo bar")
+-- do not restrict here, since '' (the root) has not yet been committed
+check(mtn("automate", "content_diff"), 0, true, true)
+check(fsize("stdout") ~= 0)
+
+-- add three more revisions and test for correct revid handling
+commit()
+R1=base_revision()
+writefile("existing", "foo foo")
+commit()
+R2=base_revision()
+writefile("existing", "foo foo bar")
+commit()
+R3=base_revision()
+
+-- one and two revisions should work
+check(mtn("automate", "content_diff", "-r", R1), 0, true, true)
+check(fsize("stdout") ~= 0)
+check(mtn("automate", "content_diff", "-r", R1, "-r", R2), 0, true, true)
+check(fsize("stdout") ~= 0)
+
+-- three and more should not
+check(mtn("automate", "content_diff", "-r", R1, "-r", R2, "-r", R3), 1, true, 
true)
+
============================================================
--- automate.cc 092f5069e1ce9988dc6260f868f3a9cd7f14e187
+++ automate.cc a6332c5bb3ceb42b9829830ae0935794086e3f2a
@@ -1499,7 +1499,7 @@ AUTOMATE(get_option, N_("OPTION"), optio
 // Arguments:
 //   1: a revision ID
 //   2: a file name
-// Added in: 3.2
+// Added in: 3.1
 // Purpose: Returns a list of revision IDs in which the content 
 // was most recently changed, relative to the revision ID specified 
 // in argument 1. This equates to a content mark following 
@@ -1556,7 +1556,7 @@ AUTOMATE(get_content_changed, N_("REV FI
 //   1: a source revision ID
 //   2: a file name (in the source revision)
 //   3: a target revision ID
-// Added in: 3.2
+// Added in: 3.1
 // Purpose: Given a the file name in the source revision, a filename 
 // will if possible be returned naming the file in the target revision. 
 // This allows the same file to be matched between revisions, accounting 
============================================================
--- cmd_diff_log.cc     0eee6f1675c850932e53b7a0214235ae0dc65ad4
+++ cmd_diff_log.cc     ad597a711af534bd952b3753d0af247f9e8c1223
@@ -222,6 +222,7 @@ dump_diffs(cset const & cs,
 dump_diffs(cset const & cs,
            app_state & app,
            bool new_is_archived,
+           std::ostream & output,
            set<split_path> const & paths,
            bool limit_paths = false)
 {
@@ -235,7 +236,7 @@ dump_diffs(cset const & cs,
       if (limit_paths && paths.find(i->first) == paths.end())
         continue;
 
-      cout << patch_sep << "\n";
+      output << patch_sep << "\n";
       data unpacked;
       vector<string> lines;
 
@@ -261,7 +262,7 @@ dump_diffs(cset const & cs,
                 i->second,
                 i->second,
                 data(), unpacked,
-                cout, app.opts.diff_format, pattern);
+                output, app.opts.diff_format, pattern);
     }
 
   map<split_path, split_path> reverse_rename_map;
@@ -283,7 +284,7 @@ dump_diffs(cset const & cs,
       file_data f_old;
       data data_old, data_new;
 
-      cout << patch_sep << "\n";
+      output << patch_sep << "\n";
 
       app.db.get_file_version(delta_entry_src(i), f_old);
       data_old = f_old.inner();
@@ -317,38 +318,33 @@ dump_diffs(cset const & cs,
                 delta_entry_src(i),
                 delta_entry_dst(i),
                 data_old, data_new,
-                cout, app.opts.diff_format, pattern);
+                output, app.opts.diff_format, pattern);
     }
 }
 
 static void
 dump_diffs(cset const & cs,
            app_state & app,
-           bool new_is_archived)
+           bool new_is_archived,
+           std::ostream & output)
 {
   set<split_path> dummy;
-  dump_diffs(cs, app, new_is_archived, dummy);
+  dump_diffs(cs, app, new_is_archived, output, dummy);
 }
 
-CMD(diff, N_("informative"), N_("[PATH]..."),
-    N_("show current diffs on stdout.\n"
-    "If one revision is given, the diff between the workspace and\n"
-    "that revision is shown.  If two revisions are given, the diff between\n"
-    "them is given.  If no format is specified, unified is used by default."),
-    options::opts::revision | options::opts::depth | options::opts::exclude
-    | options::opts::diff_options)
+// common functionality for diff and automate content_diff to determine
+// revisions and rosters which should be diffed
+static void 
+prepare_diff(cset & included,
+             app_state & app, 
+             std::vector<utf8> args,
+             bool & new_is_archived,
+             std::string & revheader)
 {
-  bool new_is_archived;
-  ostringstream header;
   temp_node_id_source nis;
+  ostringstream header;
+  cset excluded;
 
-  if (app.opts.external_diff_args_given)
-    N(app.opts.diff_format == external_diff,
-      F("--diff-args requires --external\n"
-        "try adding --external or removing --diff-args?"));
-
-  cset included, excluded;
-
   // initialize before transaction so we have a database to work with.
 
   if (app.opts.revision_selectors.size() == 0)
@@ -356,6 +352,9 @@ CMD(diff, N_("informative"), N_("[PATH].
   else if (app.opts.revision_selectors.size() == 1)
     app.require_workspace();
 
+  N(app.opts.revision_selectors.size() <= 2,
+    F("more than two revisions given"));
+  
   if (app.opts.revision_selectors.size() == 0)
     {
       roster_t new_roster, old_roster;
@@ -457,10 +456,31 @@ CMD(diff, N_("informative"), N_("[PATH].
     }
   else
     {
-      throw usage(name);
+      I(false);
     }
 
+    revheader = header.str();
+}
 
+CMD(diff, N_("informative"), N_("[PATH]..."),
+    N_("show current diffs on stdout.\n"
+    "If one revision is given, the diff between the workspace and\n"
+    "that revision is shown.  If two revisions are given, the diff between\n"
+    "them is given.  If no format is specified, unified is used by default."),
+    options::opts::revision | options::opts::depth | options::opts::exclude
+    | options::opts::diff_options)
+{
+  if (app.opts.external_diff_args_given)
+    N(app.opts.diff_format == external_diff,
+      F("--diff-args requires --external\n"
+        "try adding --external or removing --diff-args?"));
+
+  cset included;
+  std::string revs;
+  bool new_is_archived;
+  
+  prepare_diff(included, app, args, new_is_archived, revs);
+  
   data summary;
   write_cset(included, summary);
 
@@ -469,7 +489,7 @@ CMD(diff, N_("informative"), N_("[PATH].
   cout << "# " << "\n";
   if (summary().size() > 0)
     {
-      cout << header.str() << "# " << "\n";
+      cout << revs << "# " << "\n";
       for (vector<string>::iterator i = lines.begin(); 
            i != lines.end(); ++i)
         cout << "# " << *i << "\n";
@@ -483,9 +503,33 @@ CMD(diff, N_("informative"), N_("[PATH].
   if (app.opts.diff_format == external_diff) {
     do_external_diff(included, app, new_is_archived);
   } else
-    dump_diffs(included, app, new_is_archived);
+    dump_diffs(included, app, new_is_archived, cout);
 }
 
+
+// Name: content_diff
+// Arguments:
+//   (optional) one or more files to include
+// Added in: 4.0
+// Purpose: Availability of mtn diff as automate command.
+//
+// Output format: Like mtn diff, but with the header part omitted (as this is
+// doubles the output of automate get_revision). If no content changes 
happened,
+// the output is empty. All file operations beside mtn add are omitted,
+// as they don't change the content of the file.
+AUTOMATE(content_diff, N_("[FILE [...]]"),
+    options::opts::revision | options::opts::depth | options::opts::exclude)
+{
+  cset included;
+  std::string dummy_header;
+  bool new_is_archived;
+  
+  prepare_diff(included, app, args, new_is_archived, dummy_header);
+  
+  dump_diffs(included, app, new_is_archived, output);
+}
+
+
 static void
 log_certs(app_state & app, revision_id id, cert_name name,
           string label, string separator,
@@ -763,8 +807,8 @@ CMD(log, N_("informative"), N_("[FILE] .
               for (edge_map::const_iterator e = rev.edges.begin();
                    e != rev.edges.end(); ++e)
                 {
-                  dump_diffs(edge_changes(e), app, true, diff_paths,
-                             !mask.empty());
+                    dump_diffs(edge_changes(e), app, true, cout,
+                              diff_paths, !mask.empty());
                 }
             }
 
============================================================
--- commands.cc 01337415288f19b925c2097f54cb3fe05fce3ced
+++ commands.cc a46a10ed30eab12e318db1db9b48d100f1009ce1
@@ -12,7 +12,9 @@
 
 #include "transforms.hh"
 #include "simplestring_xform.hh"
+#include "localized_file_io.hh"
 #include "charset.hh"
+#include "diff_patch.hh"
 #include "inodeprint.hh"
 #include "cert.hh"
 #include "ui.hh"
@@ -24,6 +26,8 @@ using std::cin;
 #endif
 
 using std::cin;
+using std::map;
+using std::ostream;
 using std::pair;
 using std::set;
 using std::string;
============================================================
--- monotone.texi       18658fe14fa458d5592b1810fc8b353551d14719
+++ monotone.texi       7212dc67eee89e3f633aaf15f44ef3e265010a92
@@ -6762,20 +6762,22 @@ @section Automation
 Each attribute stanza also contains another entry which tells the status
 of attribute. This entry can have one of the following four values:
 
address@hidden
address@hidden
 'added': the attribute has just been added to the file
-
address@hidden
 'dropped': the attribute has just been dropped from the file
-
address@hidden
 'unchanged': the attribute has not been changed since the last revision
-
address@hidden
 'changed': the attribute has been changed since the last revision
address@hidden itemize
 
 The status 'changed' can come up if an attribute foo has been dropped and
 added afterwards with another value, like
 
 @verbatim
-# foo has the value "bar"
-$ mtn attr drop foo ; mtn attr set foo baz
+$ mtn attr drop file.txt foo ; mtn attr set file.txt foo baz
 @end verbatim
 
 If an attribute has been dropped, the output will still return the previously
@@ -6802,6 +6804,69 @@ @section Automation
 
 @end table
 
address@hidden mtn automate content_diff address@hidden address@hidden 
address@hidden ...]
+
address@hidden @strong
address@hidden Arguments:
+
+One or more @var{file} arguments restrict the diff output to these files,
+otherwise all changed files in the given revision(s) and/or current workspace
+are considered.
+
+If zero or more revisions are given, the command behaves as follows:
+
address@hidden
address@hidden
+no revision: the diff is done between the workspace revision and the 
+parent (base) revision of this workspace
address@hidden
+one revision: the diff is done between the workspace revision and the
+given revision @option{id1},
address@hidden
+two revisions: the diff is done between @option{id1} and @option{id2}; no
+workspace is needed in this case.
address@hidden itemize
+
address@hidden Added in:
+
+4.0
+
address@hidden Purpose:
+
+Prints the content changes between two revisions or a revision and the current
+workspace. This command differs from @command{mtn diff} in that way that it 
only
+outputs content changes and keeps quite on renames or drops, as the header of
address@hidden diff} is omitted (this is what @command{mtn automate 
get_revision} 
+already provides).
+
address@hidden Sample output:
+
address@hidden
+============================================================
+--- guitone/res/i18n/guitone_de.ts      
9857927823e1d6a0339b531c120dcaadd22d25e9
++++ guitone/res/i18n/guitone_de.ts      
0b4715dc296b1955b0707923d45d79ca7769dd3f
+@@ -1,6 +1,14 @@
+ <?xml version="1.0" encoding="utf-8"?>
+ <!DOCTYPE TS><TS version="1.1">
+ <context>
++    <name>AncestryGraph</name>
++    <message>
+[...]
address@hidden verbatim
+
address@hidden Output format:
+
+The GNU unified diff format. If there have been no content changes, the output
+is empty.
+
address@hidden Error conditions:
+
+If more than two revisions are given or a workspace is required, but
+not found, prints to stderr and exits with status 1. If one or more file 
+restrictions can't be applied, the command prints to stderr and exits as well.
+
address@hidden table
+
 @item mtn automate get_file @var{id}
 
 @table @strong
============================================================
--- testsuite.lua       67b5f8eb1b321452edafe257ac5f61e82c85c856
+++ testsuite.lua       ee3dadaa84390b8be6919ce41c9c5e6c4e3b4881
@@ -425,7 +425,7 @@ table.insert(tests, "db_load_must_create
 table.insert(tests, "update_no-ops_when_no_parent_revision")
 table.insert(tests, "branch-based_checkout")
 table.insert(tests, "db_load_must_create_a_new_db")
-table.insert(tests, "automate_automate_version")
+table.insert(tests, "automate_interface_version")
 table.insert(tests, "automate_heads")
 table.insert(tests, "merge_normalization_edge_case")
 table.insert(tests, "(todo)_undo_update_command")
@@ -687,3 +687,4 @@ table.insert(tests, "add_ignored")
 table.insert(tests, "db_check_(heights)")
 table.insert(tests, "disapproving_with_message")
 table.insert(tests, "add_ignored")
+table.insert(tests, "automate_content_diff")