Johannes Schindelin <[email protected]> writes:
>> + if (!keep_empty && is_empty)
>> strbuf_addf(&buf, "%c ", comment_line_char);
We are not trying to preserve an empty one, and have found an empty
one, so we comment it out, and then...
>> + if (is_empty || !(commit->object.flags & PATCHSAME)) {
>
> May I suggest inverting the logic here, to make the code more obvious and
> also to avoid indenting the block even further?
>
> if (!is_empty && (commit->object.flags & PATCHSAME))
> continue;
... if a non-empty one that already appears in the upstream, we do
not do anything to it. There is no room for keep-empty or lack of
it to affect what happens to these commits.
Otherwise the insn is emitted for the commit.
>> + strbuf_addf(&buf, "%s %s ", insn,
>> + oid_to_hex(&commit->object.oid));
>> + pretty_print_commit(&pp, commit, &buf);
>> + strbuf_addch(&buf, '\n');
>> + fputs(buf.buf, out);
>> + }
I tend to agree that the suggested structure is easier to follow
than Phillip's version.
But I wonder if this is even easier to follow. It makes it even
more clear that patchsame commits that are not empty are discarded
unconditionally.
while ((commit = get_revision(&revs))) {
int is_empty = is_original_commit_empty(commit);
if (!is_empty && (commit->object.flags & PATCHSAME))
continue;
strbuf_reset(&buf);
if (!keep_empty && is_empty)
strbuf_addf(&buf, "%c ", comment_line_char);
strbuf_addf(&buf, "%s %s ", insn,
oid_to_hex(&commit->object.oid));
pretty_print_commit(&pp, commit, &buf);
strbuf_addch(&buf, '\n');
fputs(buf.buf, out);
}
Or did I screw up the rewrite?
