clerk report: allow to choose diff command

Also, detect non-GNU diff or absence of colors, and fallback to a basic view;
this is actually more readable in logs or diffs.

build_system/clerk_driver.ml

@@ -226,15 +226,18 @@ module Cli = struct
                 ~doc:"Display the full list of tests that have been run" );
           ])
 
-  let use_patdiff =
+  let diff_command =
     Arg.(
       value
-      & flag
-      & info ["patdiff"]
-          ~env:(Cmd.Env.info "CATALA_USE_PATDIFF")
+      & opt ~vopt:(Some None) (some (some string)) None
+      & info ["diff"]
+          ~env:(Cmd.Env.info "CATALA_DIFF_COMMAND")
           ~doc:
-            "Enable use of the 'patdiff' command for showing test failure \
-             details (no effect if the command is not available)")
+            "Use a standard $(i,diff) command instead of the default \
+             side-by-side view. If no argument is supplied, the command will \
+             be $(b,patdiff) if available or $(b,diff) otherwise. A supplied \
+             argument will be used as diff command with arguments pointing to \
+             the reference file and the output file")
 
   let ninja_flags =
     let env =
@@ -915,10 +918,10 @@ let test_cmd =
       (reset_test_outputs : bool)
       (test_flags : string list)
       verbosity
-      (use_patdiff : bool)
+      (diff_command : string option option)
       (ninja_flags : string list) =
     set_report_verbosity verbosity;
-    Clerk_report.set_display_flags ~use_patdiff ();
+    Clerk_report.set_display_flags ~diff_command ();
     ninja_init ~extra:Seq.empty ~test_flags
     @@ fun build_dir nin_file ->
     let targets =
@@ -988,7 +991,7 @@ let test_cmd =
       $ Cli.reset_test_outputs
       $ Cli.test_flags
       $ Cli.report_verbosity
-      $ Cli.use_patdiff
+      $ Cli.diff_command
       $ Cli.ninja_flags)
 
 let run_cmd =
@@ -1051,11 +1054,11 @@ let runtest_cmd =
       $ Cli.single_file)
 
 let report_cmd =
-  let run color debug verbosity use_patdiff build_dir file =
+  let run color debug verbosity diff_command build_dir file =
     let _options = Catala_utils.Global.enforce_options ~debug ~color () in
     let build_dir = Option.value ~default:"_build" build_dir in
     set_report_verbosity verbosity;
-    Clerk_report.set_display_flags ~use_patdiff ();
+    Clerk_report.set_display_flags ~diff_command ();
     let open Clerk_report in
     let tests = read_many file in
     let success = summary ~build_dir tests in
@@ -1071,7 +1074,7 @@ let report_cmd =
       $ Cli.Global.color
       $ Cli.Global.debug
       $ Cli.report_verbosity
-      $ Cli.use_patdiff
+      $ Cli.diff_command
       $ Cli.build_dir
       $ Cli.single_file)
 

build_system/clerk_report.ml

@@ -33,22 +33,22 @@ type disp_flags = {
   mutable files : [ `All | `Failed | `None ];
   mutable tests : [ `All | `FailedFile | `Failed | `None ];
   mutable diffs : bool;
-  mutable use_patdiff : bool;
+  mutable diff_command : string option option;
 }
 
 let disp_flags =
-  { files = `Failed; tests = `FailedFile; diffs = true; use_patdiff = false }
+  { files = `Failed; tests = `FailedFile; diffs = true; diff_command = None }
 
 let set_display_flags
     ?(files = disp_flags.files)
     ?(tests = disp_flags.tests)
     ?(diffs = disp_flags.diffs)
-    ?(use_patdiff = disp_flags.use_patdiff)
+    ?(diff_command = disp_flags.diff_command)
     () =
   disp_flags.files <- files;
   disp_flags.tests <- tests;
   disp_flags.diffs <- diffs;
-  disp_flags.use_patdiff <- use_patdiff
+  disp_flags.diff_command <- diff_command
 
 let write_to f file =
   File.with_out_channel f (fun oc -> Marshal.to_channel oc (file : file) [])
@@ -81,20 +81,14 @@ let longuest_common_prefix_length s1 s2 =
   aux 0
 
 let diff_command =
+  let has_gnu_diff () =
+    File.process_out ~check_exit:ignore "diff" ["--version"]
+    |> Re.(execp (compile (str "GNU")))
+  in
   lazy
     begin
-      if
-        disp_flags.use_patdiff
-        && has_command "patdiff"
-        && Message.has_color stdout
-      then
-        ( ["patdiff"; "-alt-old"; "expected"; "-alt-new"; "result"],
-          fun ppf s ->
-            s
-            |> String.split_on_char '\n'
-            |> List.filter (( <> ) "")
-            |> Format.pp_print_list Format.pp_print_string ppf )
-      else
+      match disp_flags.diff_command with
+      | None when Message.has_color stdout && has_gnu_diff () ->
         let width = Message.terminal_columns () - 5 in
         ( [
             "diff";
@@ -147,6 +141,21 @@ let diff_command =
                          (String.sub r w (String.length r - w))
                      | _ -> Format.pp_print_string ppf li))
                  ppf )
+      | Some cmd_opt | (None as cmd_opt) ->
+        let command =
+          match cmd_opt with
+          | Some str -> String.split_on_char ' ' str
+          | None ->
+            if Message.has_color stdout && has_command "patdiff" then
+              ["patdiff"; "-alt-old"; "Reference"; "-alt-new"; "Result"]
+            else ["diff"; "-u"; "-L"; "Reference"; "-L"; "Result"]
+        in
+        ( command,
+          fun ppf s ->
+            s
+            |> String.trim_end
+            |> String.split_on_char '\n'
+            |> Format.pp_print_list Format.pp_print_string ppf )
     end
 
 let print_diff ppf p1 p2 =

build_system/clerk_report.mli

@@ -43,6 +43,6 @@ val set_display_flags :
   ?files:[ `All | `Failed | `None ] ->
   ?tests:[ `All | `FailedFile | `Failed | `None ] ->
   ?diffs:bool ->
-  ?use_patdiff:bool ->
+  ?diff_command:string option option ->
   unit ->
   unit

catala.opam

@@ -47,7 +47,6 @@ depends: [
   "conf-npm" {cataladevmode}
   "conf-python-3-dev" {cataladevmode}
   "cpdf" {cataladevmode}
-  "conf-diffutils" {cataladevmode}
   "conf-pandoc" {cataladevmode}
   "z3" {catalaz3mode}
   "conf-ninja"