From eecf03ffbc2d62f1739386b236cd4279a6886cef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Lebleu?= <jerome@maroufle.fr>
Date: Mon, 11 Apr 2016 18:50:45 +0200
Subject: [PATCH] [enh] Allow to show the diff between conf in
 service_regen_conf

---
 data/actionsmap/yunohost.yml |  4 +++
 src/yunohost/service.py      | 64 ++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml
index 83a83ad92..faeae2367 100644
--- a/data/actionsmap/yunohost.yml
+++ b/data/actionsmap/yunohost.yml
@@ -969,6 +969,10 @@ service:
                     help: Services name to regenerate configuration of
                     nargs: "*"
                     metavar: NAME
+                -d:
+                    full: --with-diff
+                    help: Show differences in case of configuration changes
+                    action: store_true
                 -f:
                     full: --force
                     help: >
diff --git a/src/yunohost/service.py b/src/yunohost/service.py
index 6b3ff335c..9c5f2e04f 100644
--- a/src/yunohost/service.py
+++ b/src/yunohost/service.py
@@ -30,8 +30,8 @@ import glob
 import subprocess
 import errno
 import shutil
-import difflib
 import hashlib
+from difflib import unified_diff
 
 from moulinette.core import MoulinetteError
 from moulinette.utils import log, filesystem
@@ -271,12 +271,13 @@ def service_log(name, number=50):
     return result
 
 
-def service_regen_conf(names=[], force=False):
+def service_regen_conf(names=[], with_diff=False, force=False):
     """
     Regenerate the configuration file(s) for a service
 
     Keyword argument:
         names -- Services name to regenerate configuration of
+        with_diff -- Show differences in case of configuration changes
         force -- Override all manual modifications in configuration files
 
     """
@@ -316,6 +317,10 @@ def service_regen_conf(names=[], force=False):
             conf_status = None
             regenerated = False
 
+            # Get the diff between files
+            conf_diff = _get_files_diff(
+                system_path, pending_path, True) if with_diff else None
+
             # Check if the conf must be removed
             to_remove = True if os.path.getsize(pending_path) == 0 else False
 
@@ -384,8 +389,9 @@ def service_regen_conf(names=[], force=False):
                     conf_status = 'modified'
 
             # Store the result
-            # TODO: Append the diff if --with-diff
             conf_result = {'status': conf_status}
+            if conf_diff is not None:
+                conf_result['diff'] = conf_diff
             if regenerated:
                 succeed_regen[system_path] = conf_result
                 conf_hashes[system_path] = new_hash
@@ -396,9 +402,7 @@ def service_regen_conf(names=[], force=False):
         if not succeed_regen and not failed_regen:
             logger.info(m18n.n('service_conf_up_to_date', service=service))
             continue
-        elif failed_regen:
-            logger.error(m18n.n('service_regenconf_failed', services=service))
-        else:
+        elif not failed_regen:
             logger.success(m18n.n('service_conf_updated', service=service))
         if succeed_regen:
             _update_conf_hashes(service, conf_hashes)
@@ -500,36 +504,38 @@ def _tail(file, n, offset=None):
     except IOError: return []
 
 
-def _get_diff(string, filename):
-    """
-    Show differences between a string and a file's content
+def _get_files_diff(orig_file, new_file, as_string=False, skip_header=True):
+    """Compare two files and return the differences
 
-    Keyword argument:
-        string -- The string
-        filename -- The file to compare with
+    Read and compare two files. The differences are returned either as a delta
+    in unified diff format or a formatted string if as_string is True. The
+    header can also be removed if skip_header is True.
 
     """
-    try:
-        with open(filename, 'r') as f:
-            file_lines = f.readlines()
+    contents = [[], []]
+    for i, path in enumerate((orig_file, new_file)):
+        try:
+            with open(path, 'r') as f:
+                contents[i] = f.readlines()
+        except IOError:
+            pass
 
-        string = string + '\n'
-        new_lines = string.splitlines(True)
-        if file_lines:
-            while '\n' == file_lines[-1]:
-                del file_lines[-1]
-        return difflib.unified_diff(file_lines, new_lines)
-    except IOError: return []
+    # Compare files and format output
+    diff = unified_diff(contents[0], contents[1])
+    if skip_header:
+        for i in range(2):
+            try:
+                next(diff)
+            except:
+                break
+    if as_string:
+        result = ''.join(line for line in diff)
+        return result.rstrip()
+    return diff
 
 
 def _calculate_hash(path):
-    """
-    Calculate the MD5 hash of a file
-
-    Keyword argument:
-        path -- The path to the file
-
-    """
+    """Calculate the MD5 hash of a file"""
     hasher = hashlib.md5()
     try:
         with open(path, 'rb') as f: