"brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> 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.