Re: [PATCH v1 2/2] convert: missing `LF will be replaced by CRLF

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

tboegi@web.de writes:

> From: Torsten Bögershausen <tboegi@web.de>
>
> When the "new safer autocrlf-handling" was introduced in c4805393, the logic
> around check_safe_crlf() should have been changed:

Should have been changed from what to what?  And instead it was
changed to which other logic?  What is the difference between the
ideal this change tries to bring in and what c4805393 did?

> Once a file has been commited with CRLF (and core.autocrlf is true),
> line endings are no longer converted by "git add" or "git checkout".
> The line endings will not normalized by Git.

s/will not/& be/;

> The user may run e.g. dos2unix and commit the file.

It is unclear what this sentence wants to say.  The user may overwrite
the file with random contents and commit it, too.  So what?

What is missing from this statement is what problem is being worked
around.  Do you mean: "Because Git does not normalize once CRLF is
in the index, if the user wants to have a normalized content in the
repository to correct that mistake, the user needs to do dos2unix to
fix it and commit the fixed result"?

> Factor out will_convert_lf_to_crlf() which returns when LF are converted
> into CRLF at checkout.
>
> Update the logic around check_safe_crlf() and do the possible CRLF-LF
> conversion in "git add".
> Simulate the checkout (and a possible LF-CRLF conversion) with help of
> will_convert_lf_to_crlf().

These two paragraphs are all "what I did to solve X", but what X is
is not very clear.

> ---

Missing sign-off.

>  convert.c            | 97 ++++++++++++++++++++++++++++++----------------------
>  t/t0027-auto-crlf.sh |  6 ++--
>  2 files changed, 60 insertions(+), 43 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index 67d69b5..077f5e6 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -189,33 +189,25 @@ static enum eol output_eol(enum crlf_action crlf_action)
>  }
>  
>  static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
> -                            struct text_stat *stats, enum safe_crlf checksafe)
> +			    struct text_stat *old_stats, struct text_stat *new_stats,
> +			    enum safe_crlf checksafe)
>  {
> -	if (!checksafe)
> -		return;
> -
> -	if (output_eol(crlf_action) == EOL_LF) {
> +	if (old_stats->crlf && !new_stats->crlf ) {

Space before ")"