Re: [PATCH 2/3] rebase -i --keep-empty: don't prune empty commits

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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?