On Tue, Mar 28, 2017 at 10:27:41AM -0700, Junio C Hamano wrote: > "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. I'm happy to make that change. I'm an experienced C programmer, so a pointer addition seems very readable and natural to me, but if you think it's better or more readable as &E2[E3], I can certainly reroll. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature