[LyX/master] Attempt on #9395 - Compare (diff) misses changes in nested insets

Pavel Sanda <[email protected]> Mon, 20 Jul 2026 01:18:32 +0000
Newsgroups gmane.editors.lyx.cvs
Message-ID <[email protected]>
commit 88f4caa2165c1f2c8a1b28a87035f49cd5020489
Author: Pavel Sanda <[email protected]>
Date:   Mon Jul 20 02:59:58 2026 +0200

    Attempt on #9395 - Compare (diff) misses changes in nested insets
    
    When the comparison on the current level signals shallow-match,
    diff_i copied back old text instead of diving into not-yet-compared
    insets. The changes in such insets were dropped.
    
    Fix: instead of just copy, we copy&return via processSnake.
    
    Unlike my original clumsy patch in trac which strikes the whole inset
    for such case, this does proper recursion to only cherry-pick changes
    and closes the problem on every nesting level.
    
    Assisted-by: Claude Opus 4.7
---
 .../compare_tests/nested_insets/diffs.expected.lyx | 44 ++++++++++++----------
 .../compare_tests/nested_insets/skip.test          |  1 -
 src/Compare.cpp                                    | 18 +++++----
 3 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/development/batchtests/compare_tests/nested_insets/diffs.expected.lyx b/development/batchtests/compare_tests/nested_insets/diffs.expected.lyx
index 9de05817ee..cf7f8b61ac 100644
--- a/development/batchtests/compare_tests/nested_insets/diffs.expected.lyx
+++ b/development/batchtests/compare_tests/nested_insets/diffs.expected.lyx
@@ -1,5 +1,5 @@
-#LyX 2.4 created this file. For more info see https://www.lyx.org/
-\lyxformat 599
+#LyX 2.6 created this file. For more info see https://www.lyx.org/
+\lyxformat 653
 \begin_document
 \begin_header
 \save_transient_properties true
@@ -55,13 +55,22 @@ knitr
 \use_indices false
 \paperorientation portrait
 \suppress_date false
-\justification true
-\use_refstyle 1
+\justification default
+\crossref_package refstyle
+\use_formatted_ref 0
 \use_minted 0
 \use_lineno 0
+\backgroundcolor none
+\fontcolor none
+\notefontcolor lightgray
+\boxbgcolor red
+\table_border_color default
+\table_odd_row_color default
+\table_even_row_color default
+\table_alt_row_colors_start 1
 \index Index
 \shortcut idx
-\color #008000
+\color #ff008000
 \end_index
 \secnumdepth 3
 \tocdepth 3
@@ -83,16 +92,18 @@ knitr
 \html_css_as_file 0
 \html_be_strict false
 \docbook_table_output 0
-\author 193470694 "Sam"
+\docbook_mathml_prefix 1
+\docbook_mathml_version 0
+\author 1660427103 "Document Comparison"
 \end_header
 
 \begin_body
 
 \begin_layout Standard
 asdf
-\change_deleted 193470694 1604722528
+\change_deleted 1660427103 1784507391
 1
-\change_inserted 193470694 1604722528
+\change_inserted 1660427103 1784507391
 2
 \change_unchanged
 
@@ -111,14 +122,11 @@ status open
 status open
 
 \begin_layout Plain Layout
-
 asdf
-\change_deleted 193470694 1604722552
-2
-\change_inserted 193470694 1604722555
+\change_deleted 1660427103 1784507391
 1
-\change_unchanged
-
+\change_inserted 1660427103 1784507391
+2
 \end_layout
 
 \end_inset
@@ -131,12 +139,10 @@ asdf
 
 \begin_layout Plain Layout
 asdf
-\change_deleted 193470694 1604722559
-2
-\change_inserted 193470694 1604722559
+\change_deleted 1660427103 1784507391
 1
-\change_unchanged
-
+\change_inserted 1660427103 1784507391
+2
 \end_layout
 
 \end_inset
diff --git a/development/batchtests/compare_tests/nested_insets/skip.test b/development/batchtests/compare_tests/nested_insets/skip.test
deleted file mode 100644
index d375105722..0000000000
--- a/development/batchtests/compare_tests/nested_insets/skip.test
+++ /dev/null
@@ -1 +0,0 @@
-Compare does not handle nested insets. See #9395
\ No newline at end of file
diff --git a/src/Compare.cpp b/src/Compare.cpp
index fe2d851349..01c5c87469 100644
--- a/src/Compare.cpp
+++ b/src/Compare.cpp
@@ -30,7 +30,7 @@
  *   1. Finds bidirectionally middle snake from ends to the middle of the given range
  *         -> L_ses - length of the shortest edit script
  *   2.
- *     a) all equal (shallow match, L_ses == 0, default for empty snake) -> copy back
+ *     a) all equal (shallow match, L_ses == 0, default for empty snake) -> inset recursion & copy back
  *     b) all different (middle_snake.o.empty) -> give back old as deleted, new as inserted
  *     c) proper snake ->  Divide and conquer:
  *      diff_i(before snake)
@@ -43,12 +43,14 @@
  *    - copy -> destination buffer
  * 
  *
- * Notes: AFAICS from Myers point of view II. & III. should be enough and it's not clear
+ * TODO:  AFAICS from Myers point of view II. & III. should be enough and it's not clear
  *        from comments or commits why bother with I. 
  *        Attempt to mind-read Vincent: If there is shallow-match right on the top level
- *        diff_i does *not* recurse into insets and would miss changes. This is fixed by 
- *        processSnake in I. -- which is just band-aid and explodes in more nested diffInset
- *        case ("very rare case" mentioned in diff_i), now tracked as #9395.
+ *        diff_i did *not* recurse into insets and would miss changes. This was fixed by
+ *        processSnake in I. -- which was just band-aid and exploded in more nested
+ *        diffInset case ("very rare case" mentioned in diff_i), tracked as #9395.
+ *        This is now fixed by processSnake recursion on L_ses == 0 and closes both
+ *        problems above. I. should be now redundant and ripe for the waste bin.
  */
  
 #include <config.h>
@@ -804,9 +806,9 @@ void Compare::Impl::diff_i(DocRangePair const & rp)
 	// the strings were completely different, or we found a middle
 	// snake and we can split the string into two parts to process.
 	if (L_ses == 0)
-		// Two the same strings (this must be a very rare case, because
-		// usually this will be part of a snake adjacent to these strings).
-		writeToDestBuffer(rp.o);
+		// Two the same strings at this level, but nested insets may still
+		// differ - processSnake recurses into them via diffInset().
+		processSnake(rp);
 
 	else if (middle_snake.o.empty()) {
 		// Two totally different strings
-- 
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs