"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> + if (!(flags & ALLOW_EMPTY)) {
> + struct commit *first_parent = current_head;
I would have used first_parent_tree variable instead here. That
way, you did not have to have an overlong ternary expression down
there, I expect.
> + if (flags & AMEND_MSG) {
> + if (current_head->parents) {
> + first_parent = current_head->parents->item;
> + if (repo_parse_commit(r, first_parent)) {
> + res = error(_("could not parse HEAD commit"));
> + goto out;
> + }
> + } else {
> + first_parent = NULL;
> + }
> + }
> + if (oideq(first_parent ? get_commit_tree_oid(first_parent) :
> + the_hash_algo->empty_tree, &tree)) {
Style. It often makes the code much easier to read when you strive
to break lines at the boundary of larger syntactic units. In this
(A ? B : C, D) parameter list, ternary A ? B : C binds much tighter
than the comma after it, so if you are breaking line inside it, you
should break line after the comma, too, i.e.
oideq(first_parent
? get_commit_tree_oid(first_parent)
: the_hash_algo->empty_tree,
&tree)
to avoid appearing if C and D have closer relationship than B and C,
which your version gives.
But I hope that it becomes a moot point, if we computed first_parent_tree
inside the new "if (flags & AMEND_MSG)" block to leave this oideq()
just
if (oideq(first_parent_tree, &tree)) {
