Calvin Wan <calvinwan@xxxxxxxxxx> writes:
>> > @@ -245,21 +269,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>> > newmode = ce_mode_from_stat(ce, st.st_mode);
>> > }
>> >
>> > - if (!changed && !dirty_submodule) {
>> > - ce_mark_uptodate(ce);
>> > - mark_fsmonitor_valid(istate, ce);
>> > - if (!revs->diffopt.flags.find_copies_harder)
>> > - continue;
>> > - }
>> > - oldmode = ce->ce_mode;
>> > - old_oid = &ce->oid;
>> > - new_oid = changed ? null_oid() : &ce->oid;
>> > - diff_change(&revs->diffopt, oldmode, newmode,
>> > - old_oid, new_oid,
>> > - !is_null_oid(old_oid),
>> > - !is_null_oid(new_oid),
>> > - ce->name, 0, dirty_submodule);
>> > -
>> > + if (diff_change_helper(&revs->diffopt, newmode, dirty_submodule,
>> > + changed, istate, ce))
>> > + continue;
>> > }
>>
>> If I'm reading the indentation correctly, the "continue" comes right
>> before the end of the for-loop block, so it's a no-op and should be
>> removed.
>
> It is a no-op, but I left it in as future-proofing in case more code is
> added after that block later. I'm not sure whether that line of
> reasoning is enough to leave it in though.
I don't think it is. If we haven't thought of the reason why we would
need to skip code, that seems like YAGNI to me.
As a matter of personal taste, I wouldn't leave a trailing "continue" in
an earlier patch even if I were going to change it in a later patch,
because it looks too much like an unintentional mistake.
