Add word based textual diff output (#131)

azzsal · web-flow · commit bbf3fb8f4b74 · 2025-03-03T11:48:33.000+01:00
* Update diff representation

* Update diff types

* Update inline tests

* Fix variant diff type

* Update textual diff output to have highlighting of exact changes in type declarations

* Add a command flag for word diff textual diff output

* Add pretty printer for word-based textual diff output

* Minor fix

* Add tests for word-based textual diffs

* Add changelog entry

* Fix spacing

* Minor fixes

* Restore old text diff representation

* Add tests for other word-based diff CLI options

* Add an optional argument to --word-diff

* Update word-based printers for the new optionalargument to --word-diff

* Minor fix
diff --git a/CHANGES.md b/CHANGES.md
@@ -8,6 +8,7 @@
 - Add detection of addition and removal of class type declarations (#103, @azzsal)
 - Add initial support for unwrapped libraries (#107, @Siddhi-agg, @azzsal)
 - Add detection of modified class declarations and class types (#106, @azzsal)
+- Add word-level display of textual diffs (#131, @azzsal)
 
 ### Changed
 
diff --git a/bin/api_diff.ml b/bin/api_diff.ml
@@ -1,6 +1,19 @@
 let tool_name = "api-diff"
 
 type mode = Unwrapped | Wrapped of string | Cmi
+type word = Plain | Color
+type display = Line | Word of word
+
+let display_mode word =
+  match word with
+  | Some "plain" -> Ok (Word Plain)
+  | Some "color" -> Ok (Word Color)
+  | None -> Ok Line
+  | _ ->
+      Error
+        "Invalid argument. --word-diff takes an optional mode arguemnt.\n\
+         The mode can be either color or plain. If no mode is provided, it \
+         defaults to plain."
 
 let both_directories reference current =
   match (Sys.is_directory reference, Sys.is_directory current) with
@@ -40,8 +53,16 @@ let mode ~reference ~current ~main_module ~unwrapped =
         "Either --main-module or --unwrapped must be provided when diffing \
          entire libraries."
 
-let run (`Main_module main_module) (`Unwrapped_library unwrapped)
-    (`Ref_cmi reference) (`Current_cmi current) =
+let print_diff text_diff display_mode =
+  match display_mode with
+  | Line -> Api_watch.Text_diff.With_colors.pp Format.std_formatter text_diff
+  | Word Plain ->
+      Api_watch.Text_diff.Word.pp ~mode:`Plain Format.std_formatter text_diff
+  | Word Color ->
+      Api_watch.Text_diff.Word.pp ~mode:`Color Format.std_formatter text_diff
+
+let run (`Word_diff word_diff) (`Main_module main_module)
+    (`Unwrapped_library unwrapped) (`Ref_cmi reference) (`Current_cmi current) =
   let open CCResult.Infix in
   let* reference_map, current_map =
     let* curr_mode = mode ~reference ~current ~main_module ~unwrapped in
@@ -72,15 +93,29 @@ let run (`Main_module main_module) (`Unwrapped_library unwrapped)
     |> List.filter_map (fun (_, v) -> v)
   in
   let has_changes = not (List.is_empty diff_map) in
+  let* display_mode = display_mode word_diff in
   List.iter
     (fun diff ->
       let text_diff = Api_watch.Text_diff.from_diff diff in
-      Api_watch.Text_diff.With_colors.pp Format.std_formatter text_diff)
+      print_diff text_diff display_mode)
     diff_map;
   if has_changes then Ok 1 else Ok 0
 
 let named f = Cmdliner.Term.(app (const f))
 
+let word_diff =
+  let docv = "MODE" in
+  let doc =
+    "Display the API diff in an inline word diff format rather than the usual \
+     line diff. Follows the same conventions as $(b,git diff --word-diff)."
+  in
+  named
+    (fun x -> `Word_diff x)
+    Cmdliner.Arg.(
+      value
+      & opt ~vopt:(Some "plain") (some string) None
+      & info ~doc ~docv [ "word-diff" ])
+
 let main_module =
   let docv = "MAIN_MODULE_NAME" in
   let doc =
@@ -128,7 +163,8 @@ let info =
 
 let term =
   Cmdliner.Term.(
-    const run $ main_module $ unwrapped_library $ ref_cmi $ current_cmi)
+    const run $ word_diff $ main_module $ unwrapped_library $ ref_cmi
+    $ current_cmi)
 
 let () =
   Fmt_tty.setup_std_outputs ();
diff --git a/lib/text_diff.ml b/lib/text_diff.ml
@@ -235,36 +235,31 @@ and process_privacy_diff privacy_diff =
   | Changed Added_p -> [ Iconflict { iorig = None; inew = Some " private" } ]
   | Changed Removed_p -> [ Iconflict { iorig = Some " private"; inew = None } ]
 
-and punctuate p s f = Printf.sprintf "%s%s" (if f then p else "") s
-
 and process_type_params_diff params_diff =
+  let open Stddiff in
   let open Diff in
   let params_hunks =
     match params_diff with
     | Same params ->
         List.mapi
-          (fun i p -> Icommon (punctuate ", " (type_expr_to_string p) (i > 0)))
+          (fun i p ->
+            let comma = if i > 0 then ", " else "" in
+            Icommon (Printf.sprintf "%s%s" comma (type_expr_to_string p)))
           params
     | Changed changed_params ->
         List.mapi
           (fun i p ->
+            let comma = if i > 0 then ", " else "" in
             match p with
-            | Stddiff.Same same_param ->
+            | Same same_param ->
                 Icommon
-                  (punctuate ", " (type_expr_to_string same_param) (i > 0))
-            | Stddiff.Changed (Added_tp p) ->
+                  (Printf.sprintf "%s%s" comma (type_expr_to_string same_param))
+            | Changed (Added_tp p) ->
                 Iconflict
-                  {
-                    iorig = None;
-                    inew = Some (punctuate ", " (type_expr_to_string p) (i > 0));
-                  }
-            | Stddiff.Changed (Removed_tp p) ->
+                  { iorig = None; inew = Some (comma ^ type_expr_to_string p) }
+            | Changed (Removed_tp p) ->
                 Iconflict
-                  {
-                    iorig =
-                      Some (punctuate ", " (type_expr_to_string p) (i > 0));
-                    inew = None;
-                  })
+                  { iorig = Some (comma ^ type_expr_to_string p); inew = None })
           changed_params
   in
   let open_paren = Icommon " (" in
@@ -470,24 +465,19 @@ and process_tuple_type_diff tuple_diff =
   let open Stddiff in
   List.mapi
     (fun i te_diff ->
+      let star = if i > 0 then " * " else "" in
       match te_diff with
       | Same same_te ->
-          [ Icommon (punctuate " * " (type_expr_to_string same_te) (i > 0)) ]
+          [ Icommon (Printf.sprintf "%s%s" star (type_expr_to_string same_te)) ]
       | Changed (Added te) ->
           [
             Iconflict
-              {
-                iorig = None;
-                inew = Some (punctuate " * " (type_expr_to_string te) (i > 0));
-              };
+              { iorig = None; inew = Some (star ^ type_expr_to_string te) };
           ]
       | Changed (Removed te) ->
           [
             Iconflict
-              {
-                iorig = Some (punctuate " * " (type_expr_to_string te) (i > 0));
-                inew = None;
-              };
+              { iorig = Some (star ^ type_expr_to_string te); inew = None };
           ]
       | Changed (Modified { reference; current }) ->
           let te_hunk =
@@ -657,3 +647,33 @@ module With_colors = struct
   let pp_diff fmt diff = pp_ printer fmt diff
   let pp fmt t = gen_pp pp_diff fmt t
 end
+
+module Word = struct
+  let pp_inline_conflict_string ~src ~mode ppf s =
+    match (src, mode) with
+    | _, `Color -> Fmt.pf ppf "%s" s
+    | `Orig, `Plain -> Fmt.pf ppf "[-%s-]" s
+    | `New, `Plain -> Fmt.pf ppf "{+%s+}" s
+
+  let pp_inline_conflict ~src ~mode ppf conflict =
+    let color = match src with `Orig -> `Red | `New -> `Green in
+    Fmt.styled color
+      (Fmt.option ~none:Fmt.nop (pp_inline_conflict_string ~src ~mode))
+      ppf conflict
+
+  let pp_inline_hunk ~mode ppf inline_hunk =
+    match inline_hunk with
+    | Icommon s -> Fmt.string ppf s
+    | Iconflict { iorig; inew } ->
+        pp_inline_conflict ~src:`Orig ~mode ppf iorig;
+        pp_inline_conflict ~src:`New ~mode ppf inew
+
+  let pp_inline_hunks ~mode ppf ihunks =
+    Fmt.pf ppf " %a\n" (Fmt.list ~sep:Fmt.nop (pp_inline_hunk ~mode)) ihunks
+
+  let printer ~mode =
+    { With_colors.printer with inline_hunks = pp_inline_hunks ~mode }
+
+  let pp_diff ~mode fmt diff = pp_ (printer ~mode) fmt diff
+  let pp ~(mode : [ `Plain | `Color ]) fmt t = gen_pp (pp_diff ~mode) fmt t
+end
diff --git a/lib/text_diff.mli b/lib/text_diff.mli
@@ -23,3 +23,9 @@ module With_colors : sig
   (** Same as regular [pp] but prints added lines in green and removed lines in
       red. *)
 end
+
+module Word : sig
+  val pp : mode:[ `Plain | `Color ] -> Format.formatter -> t -> unit
+  (** Pretty-print the text diff in an inlined word diff format, similar to
+      [git diff --word-diff=[<mode>]] *)
+end
diff --git a/tests/api-diff/word_based_diff_tests.t b/tests/api-diff/word_based_diff_tests.t
@@ -0,0 +1,67 @@
+Here we generate a `.mli` file with some types:
+
+  $ cat > ref.mli << EOF
+  > type t = int
+  > type ('a, 'b, 'c) u = { mutable a : 'a; b : 'b; c : 'c }
+  > type v = A of int * int | B of { a : int; b : float }
+  > type 'a p = 'a * 'a
+  > EOF
+
+We generate the .cmi file
+
+  $ ocamlc ref.mli
+
+Changing the types in the ref.mli
+
+  $ cat > cur.mli << EOF
+  > type t
+  > type ('a) u = { a : 'a; b : int; }
+  > type v = A of { a : int; b : int } | B of { a : int; b : string }
+  > type p = int * int
+  > EOF
+
+We generate the .cmi file
+
+  $ ocamlc cur.mli
+
+Run api-watcher on the two cmi file with default word-level diffing option enabled
+
+  $ api-diff ref.cmi cur.cmi --word-diff
+  diff module Cur:
+   type [-'a-] p = [-'a * 'a-]{+int * int+}
+   type t[- =-][- int-]
+   type ('a[-, 'b-][-, 'c-]) u =
+     {[- mutable-] a : 'a; b : [-'b-]{+int+};[- c : 'c;-] }
+   type v =
+     | A of [-int * int-]{+{ a : int; b : int; }+}
+     | B of { a : int; b : [-float-]{+string+}; }
+  
+  [1]
+
+Run api-watcher on the two cmi file with plain word-level diffing option enabled
+
+  $ api-diff --word-diff=plain ref.cmi cur.cmi
+  diff module Cur:
+   type [-'a-] p = [-'a * 'a-]{+int * int+}
+   type t[- =-][- int-]
+   type ('a[-, 'b-][-, 'c-]) u =
+     {[- mutable-] a : 'a; b : [-'b-]{+int+};[- c : 'c;-] }
+   type v =
+     | A of [-int * int-]{+{ a : int; b : int; }+}
+     | B of { a : int; b : [-float-]{+string+}; }
+  
+  [1]
+
+Run api-watcher on the two cmi file with color word-level diffing option enabled
+
+  $ api-diff --word-diff=color ref.cmi cur.cmi
+  diff module Cur:
+   type 'a p = 'a * 'aint * int
+   type t = int
+   type ('a, 'b, 'c) u =
+     { mutable a : 'a; b : 'bint; c : 'c; }
+   type v =
+     | A of int * int{ a : int; b : int; }
+     | B of { a : int; b : floatstring; }
+  
+  [1]
