On Sat, Sep 03, 2022 at 09:39:12AM +0200, Stefan Sperling wrote:
> On Tue, Aug 30, 2022 at 02:47:58PM +0100, Tom Jones wrote:
> > Prior to this change ed script output was in the wrong order, i.e. in
> > the order diff_result provides and changes where missing added or
> > changed lines. ed edits need to be in reverse order to keep the edited
> > file in sync 
> 
> The onnly blocking problem with this patch I can see is that output
> line offsets aren't being filled in correctly.
> 
> For example:
> 
>  +			rc = fprintf(dest, "%d,%dc\n", left_start, cc->left.end);
>  +		}
>  +	}
> 
>  ... here, 'rc' should be used to update the line offsets array.
> 
>  +
>  +	/* Now write out the new lines in all the joined chunks. */

Hi,

Following patch is updated to update the line offsets in the same way
that diff_output_plain.c does.

- Tom
diff --git a/lib/diff_output_edscript.c b/lib/diff_output_edscript.c
index 42d4d5b..9668398 100644
--- a/lib/diff_output_edscript.c
+++ b/lib/diff_output_edscript.c
@@ -32,9 +32,9 @@ static int
 output_edscript_chunk(struct diff_output_info *outinfo,
     FILE *dest, const struct diff_input_info *info,
     const struct diff_result *result,
-    struct diff_chunk_context *cc)
+    struct diff_chunk_context *cc, off_t *outoff)
 {
-	off_t outoff = 0, *offp;
+	off_t *offp;
 	int left_start, left_len, right_start, right_len;
 	int rc;
 
@@ -81,19 +81,43 @@ output_edscript_chunk(struct diff_output_info *outinfo,
 		}
 	} else {
 		/* change */
-		if (left_len == 1 && right_len == 1) {
-			rc = fprintf(dest, "%dc%d\n", left_start, right_start);
-		} else if (left_len == 1) {
-			rc = fprintf(dest, "%dc%d,%d\n", left_start,
-			    right_start, cc->right.end);
-		} else if (right_len == 1) {
-			rc = fprintf(dest, "%d,%dc%d\n", left_start,
-			    cc->left.end, right_start);
+		if (left_len == 1) {
+			rc = fprintf(dest, "%dc\n", left_start);
 		} else {
-			rc = fprintf(dest, "%d,%dc%d,%d\n", left_start,
-			    cc->left.end, right_start, cc->right.end);
+			rc = fprintf(dest, "%d,%dc\n", left_start, cc->left.end);
 		}
 	}
+	if (rc < 0)
+		return errno;
+	if (outinfo) {
+		ARRAYLIST_ADD(offp, outinfo->line_offsets);
+		if (offp == NULL)
+			return ENOMEM;
+		*outoff += rc;
+		*offp = *outoff;
+	}
+
+	/* Now write out the new lines in all the joined chunks. */
+	int c_idx;
+	for (c_idx = cc->chunk.start; c_idx < cc->chunk.end; c_idx++) {
+		const struct diff_chunk *c = &result->chunks.head[c_idx];
+		if (c->left_count && !c->right_count)
+				continue;
+		if (c->right_count && !c->left_count) {
+			rc = diff_output_lines(outinfo, dest,
+					  c->solved ? "" : "?",
+					  c->right_start, c->right_count);
+			fprintf(dest, ".\n");
+		}
+		if (rc)
+			return rc;
+		if (cc->chunk.end == result->chunks.len) {
+			rc = diff_output_trailing_newline_msg(outinfo, dest, c);
+			if (rc != DIFF_RC_OK)
+				return rc;
+		}
+	}
+
 	if (rc < 0)
 		return errno;
 	if (outinfo) {
@@ -101,7 +125,7 @@ output_edscript_chunk(struct diff_output_info *outinfo,
 		if (offp == NULL)
 			return ENOMEM;
 		outoff += rc;
-		*offp = outoff;
+		*offp = *outoff;
 	}
 
 	return DIFF_RC_OK;
@@ -121,6 +145,7 @@ diff_output_edscript(struct diff_output_info **output_info,
 	bool force_text = (flags & DIFF_FLAG_FORCE_TEXT_DATA);
 	bool have_binary = (atomizer_flags & DIFF_ATOMIZER_FOUND_BINARY_DATA);
 	int i, rc;
+	off_t outoff = 0, *offp;
 
 	if (!result)
 		return EINVAL;
@@ -151,7 +176,11 @@ diff_output_edscript(struct diff_output_info **output_info,
 		return DIFF_RC_OK;
 	}
 
-	for (i = 0; i < result->chunks.len; i++) {
+	/*
+	 * Changes in an ed script are in reverse order, the last change to the
+	 * file has to be made first to keep state while making edits.
+	 */
+	for (i = result->chunks.len-1; i >= 0 ; i--) {
 		struct diff_chunk *chunk = &result->chunks.head[i];
 		enum diff_chunk_type t = diff_chunk_type(chunk);
 		struct diff_chunk_context next;
@@ -178,13 +207,15 @@ diff_output_edscript(struct diff_output_info **output_info,
 			continue;
 		}
 
-		rc = output_edscript_chunk(outinfo, dest, info, result, &cc);
+		rc = output_edscript_chunk(outinfo, dest, info, result, &cc,
+			&outoff);
 		if (rc != DIFF_RC_OK)
 			return rc;
 		cc = next;
 	}
 
 	if (!diff_chunk_context_empty(&cc))
-		return output_edscript_chunk(outinfo, dest, info, result, &cc);
+		return output_edscript_chunk(outinfo, dest, info, result, &cc,
+			&outoff);
 	return DIFF_RC_OK;
 }

• updated-edscript.diff (4K)