Re: [PATCH v2 16/21] Make sha1_array_append take a struct object_id *

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

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Convert the callers to pass struct object_id by changing the function
> declaration and definition and applying the following semantic patch:
>
> @@
> expression E1, E2, E3;
> @@
> - sha1_array_append(E1, E2[E3].hash)
> + sha1_array_append(E1, E2 + E3)
>
> @@
> expression E1, E2;
> @@
> - sha1_array_append(E1, E2.hash)
> + sha1_array_append(E1, &E2)

I noticed something similar in the change to bisect.c while reading
the previous step, and I suspect that the above two rules leave
somewhat inconsistent and harder-to-read result.  Wouldn't it make
the result more readable if the former rule were

    -sha1_array_append(E1, E2[E3].hash)
    +sha1_array_append(E1, &E2[E3])


FWIW, the bit that made me read it twice in the previous step was
this change

-		strbuf_addstr(&joined_hexs, sha1_to_hex(array->sha1[i]));
+		strbuf_addstr(&joined_hexs, oid_to_hex(array->oid + i));

which I would have written &(array->oid[i]) instead.

After all, the original written by a human said E2[E3].hash (or
array->sha1[i]) because to the human's mind, E2 is a series of
things that can be indexed with an int E3, and even though 

    *(E2 + E3)
    E2[E3]
    E3[E2]

all mean the same thing, the human decided that E2[E3] is the most
natural way to express this particular reference to an item in the
array.  &E2[E3] would keep that intention by the original author
better than E2 + E3.

The above comment does not affect the correctness of the conversion,
but I think it would affect the readability of the resulting code.