Re: [PATCH v2 3/7] Allow core.autocrlf=input and core.eol=crlf

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

tboegi@web.de writes:

> From: Torsten Bögershausen <tboegi@web.de>
>
> Commit  942e77 "Add "core.eol" config variable" adds a condition to
> the config parser: core.autocrlf=input is not allowed together with
> core.eol=crlf.
>
> This may lead to unnecessary problems, when config files are parsed:
> A global config file sets core.autocrlf=input,
> the repo local config file sets is own configuration: core.eol=crlf

This paragraph is unparsable.  Do you mean

	When the global configuration file sets core.autocrlf=input
        and the repository local one sets core.eol=crlf, they violate
        this condition and triggers an error when the configuration
        is read.

I think that is the designed behaviour, so it is not "a problem" at
all, and an example that makes them come from two different sources
of configuration is a red herring.  The parser faithfully implement
the policy that the combination is invalid.

The "problem" is in the policy itself, which you address below.

> There is no need to prevent combinations in config.c, in any case
> core.autocrlf overrides core.eol.

So

        Even though the configuration parser errors out when
        core.autocrlf is set to 'input' when core.eol is set to
        'crlf', there is no need to do so, because the core.autocrlf
        setting trumps core.eol.

or something like that, perhaps?

> Allow all combinations of core.crlf and core.eol and document
> that core.autocrlf overrides core.eol.

That is a good change.

> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
>  Documentation/config.txt | 6 +++---
>  config.c                 | 4 ----
>  2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 2cd6bdd..4a27ad4 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -337,9 +337,9 @@ core.quotePath::
>  
>  core.eol::
>  	Sets the line ending type to use in the working directory for
> -	files that have the `text` property set.  Alternatives are
> -	'lf', 'crlf' and 'native', which uses the platform's native
> -	line ending.  The default value is `native`.  See
> +	files that have the `text` property set when core.autocrlf is false.
> +	Alternatives are 'lf', 'crlf' and 'native', which uses the platform's
> +	native line ending.  The default value is `native`.  See
>  	linkgit:gitattributes[5] for more information on end-of-line
>  	conversion.
>  
> diff --git a/config.c b/config.c
> index 9ba40bc..a6adc8b 100644
> --- a/config.c
> +++ b/config.c
> @@ -803,8 +803,6 @@ static int git_default_core_config(const char *var, const char *value)
>  
>  	if (!strcmp(var, "core.autocrlf")) {
>  		if (value && !strcasecmp(value, "input")) {
> -			if (core_eol == EOL_CRLF)
> -				return error("core.autocrlf=input conflicts with core.eol=crlf");
>  			auto_crlf = AUTO_CRLF_INPUT;
>  			return 0;
>  		}
> @@ -830,8 +828,6 @@ static int git_default_core_config(const char *var, const char *value)
>  			core_eol = EOL_NATIVE;
>  		else
>  			core_eol = EOL_UNSET;
> -		if (core_eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT)
> -			return error("core.autocrlf=input conflicts with core.eol=crlf");
>  		return 0;
>  	}