Re: [PATCH 1/2] read-cache: do not trust a size change when conversion is active

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

Curtis Allen Smith <curtis.allen.smith@gmail.com> writes:

> "git status" can report a file as modified while "git diff" and
> ...
> next run of the tool flags everything again.
>
> Signed-off-by: Curtis Allen Smith <curtis.allen.smith@gmail.com>
> ---

That's overly verbose.

>  read-cache.c    | 129 ++++++++++++++++++++++++++++++++++++++++++++++--
>  t/t0020-crlf.sh |  45 +++++++++++++++++
>  2 files changed, 171 insertions(+), 3 deletions(-)

And it is curious why we need so much new code, especially after
reading an explaination in the proposed log message that makes it
sound as if "we let ce_modified_check_fs() to compare converted
result already when timestamps differ, and it is just the matter of
doing the same when sizes are the same" is what is happening in the
patch.  Why do we need to add a new function that compares converted
data?  A new function is not automatically a bad thing.  If there is
already an existing code path that does the same thing, a new
function may be a good way to replace that code path with a more
generic code and apply essentially the same logic implemented by
that new more generic code to a new code path.  But in such a
refactoring patch, we usually see a comparable number of removed
lines, which is not what we see in the diffstat above.