[OT] perhaps we want to support copied-context diff output

[Junio C Hamano <gitster@pobox.com>]

The patch looks correct, but I have an offtopic comment that does not
have anything to do with the problem being discussed right now.

Nicolas Pitre <nico@cam.org> writes:

> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 4f44658..5002cc6 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -1245,28 +1245,37 @@ static void get_object_details(void)
> ...
>  static int type_size_sort(const void *_a, const void *_b)
>  {
>  	const struct object_entry *a = *(struct object_entry **)_a;
>  	const struct object_entry *b = *(struct object_entry **)_b;
>  
> -	if (a->type < b->type)
> -		return -1;
>  	if (a->type > b->type)
> -		return 1;
> -	if (a->hash < b->hash)
>  		return -1;
> -	if (a->hash > b->hash)
> +	if (a->type < b->type)
>  		return 1;
> -	if (a->preferred_base < b->preferred_base)
> +	if (a->hash > b->hash)
>  		return -1;
> -	if (a->preferred_base > b->preferred_base)
> +	if (a->hash < b->hash)
>  		return 1;
> -	if (a->size < b->size)
> +	if (a->preferred_base > b->preferred_base)
>  		return -1;
> +	if (a->preferred_base < b->preferred_base)
> +		return 1;
>  	if (a->size > b->size)
> +		return -1;
> +	if (a->size < b->size)
>  		return 1;
> -	return a > b ? -1 : (a < b);  /* newest last */
> +	return a < b ? -1 : (a > b);  /* newest first */
>  }

Before being able to understand what was going on, I had to shuffle the
above patch by duplicating the context lines, prefix them with '-' and
then '+', and grouping preimage lines and postimage lines together, to
come up with this patch:

 static int type_size_sort(const void *_a, const void *_b)
 {
 	const struct object_entry *a = *(struct object_entry **)_a;
 	const struct object_entry *b = *(struct object_entry **)_b;
 
-	if (a->type < b->type)
-		return -1;
-	if (a->type > b->type)
-		return 1;
-	if (a->hash < b->hash)
-		return -1;
-	if (a->hash > b->hash)
-		return 1;
-	if (a->preferred_base < b->preferred_base)
-		return -1;
-	if (a->preferred_base > b->preferred_base)
-		return 1;
-	if (a->size < b->size)
-		return -1;
-	if (a->size > b->size)
-		return 1;
-	return a > b ? -1 : (a < b);  /* newest last */
+	if (a->type > b->type)
+		return -1;
+	if (a->type < b->type)
+		return 1;
+	if (a->hash > b->hash)
+		return -1;
+	if (a->hash < b->hash)
+		return 1;
+	if (a->preferred_base > b->preferred_base)
+		return -1;
+	if (a->preferred_base < b->preferred_base)
+		return 1;
+	if (a->size > b->size)
+		return -1;
+	if (a->size < b->size)
+		return 1;
+	return a < b ? -1 : (a > b);  /* newest first */
 }

Perhaps we may want to add "diff -c" (copied context) output format as
an option, which may be easier to read.

A possible alternative with much less impact to our toolset would be to
stay with unified context format but employ some heuristics (e.g. "a
hunk has many small context lines between preimage and postimage pairs")
and rewrite the diff output automatically like what I did by hand above.

The problematic region has 26 lines, among which 9 lines are deleted
material and 9 lines are added material, and it contains 8 isolated
groups of unchanged material, one line each.  A heuristics to notice
such excessively large number of groups of unchanged lines compared to
the size of the hunk itself would be reasonably easy to implement.