Re: [PATCH 1/2] convert: add a helper to determine the correct EOL for a given path

["Torsten Bögershausen" <tboegi@web.de>]

On 2016-01-22 18.01, Johannes Schindelin wrote:
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  convert.c | 29 +++++++++++++++++++++++++++++
>  convert.h |  2 ++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/convert.c b/convert.c
> index 814e814..b458734 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -758,6 +758,35 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
>  	}
>  }
>  
> +enum eol eol_for_path(const char *path, const char *src, size_t len)
> +{
> +	struct conv_attrs ca;
> +	struct text_stat stats;
> +
> +	if (!path) {
I don't think this is ideal.
When path is NULL, can we set it to "*" to catch .gitattributes like
"* text=auto"
?
> +		memset(&ca, 0, sizeof(ca));
> +		ca.crlf_action = CRLF_AUTO;
> +		ca.eol_attr = EOL_UNSET;
> +	} else {
> +		convert_attrs(&ca, path);
> +		if (ca.eol_attr == EOL_UNSET)
> +			ca.eol_attr = output_eol(ca.crlf_action);
> +		if (ca.eol_attr != EOL_UNSET)
> +			return ca.eol_attr;
This doesn't seem to be correct.
When "File -text" is set, no CRLF needs to be added.
> +	}
> +	if (!len || (ca.crlf_action != CRLF_AUTO &&
> +				ca.crlf_action != CRLF_GUESS))
Currently core.autocrlf = true overrules "core.eol".

See the line in t0027:
#              core.eol  core.autocrlf
checkout_files    lf      true   ""       CRLF  CRLF  CRLF_mix_LF  LF_mix_CR
LF_nul

Letting core.autocrlf overrule core.eol is not documented very well,
I have it on my todo-list. But I think it makes sense to keep the behavour.

I'm currently working on simplification of convert.c, so a funtion like this
is really a step forward.
Does it makes sense to add a comment, that some corner cases may
need additional tweaking ?