unclear docs

unclear docs

[Sergey Lukashev]

Hello!

Could somebody please explain what 'core.autocrlf overrides core.eol' means given that we have the following statement in core.eol docs:
"Sets the line ending type to use in the working directory for files that have the text property set when core.autocrlf is false".

I would like to know which line endings I'm supposed to get in the working tree if I have core.autocrlf=false and '* text=auto' in .gitattributes on windows. I get CRLFs  which is consistent with the last quote but I don't understand what 'core.autocrlf overrides core.eol' means in this case.

Links:
https://git-scm.com/docs/git-config#git-config-coreeol
https://git-scm.com/docs/gitattributes#_checking_out_and_checking_in

Re: unclear docs

[Sergey Lukashev]

A follow up on my previous mail.

Well, I have two problems:
1) The endings I get with core.autocrlf=false depend on whether I have * text=auto (a file was commited with LFs). At least in git 2.20.1
2) If the quote holds true then autocrlf=false is actually "true" (for output conversion) on Windows by default because default core.eol is 'native', which is CRLF. I believe autocrlf=false is supposed to mean "no any conversion, input or output, read or write the repo as is".

27.01.2019, 13:05, "Sergey Lukashev" <lukashev.s@ya.ru>:
> Hello!
>
> Could somebody please explain what 'core.autocrlf overrides core.eol' means given that we have the following statement in core.eol docs:
> "Sets the line ending type to use in the working directory for files that have the text property set when core.autocrlf is false".
>
> I would like to know which line endings I'm supposed to get in the working tree if I have core.autocrlf=false and '* text=auto' in .gitattributes on windows. I get CRLFs which is consistent with the last quote but I don't understand what 'core.autocrlf overrides core.eol' means in this case.
>
> Links:
> https://git-scm.com/docs/git-config#git-config-coreeol
> https://git-scm.com/docs/gitattributes#_checking_out_and_checking_in

Re: unclear docs

[Jeff King]

On Sun, Jan 27, 2019 at 02:55:23PM +0300, Sergey Lukashev wrote:

> Well, I have two problems:
> 1) The endings I get with core.autocrlf=false depend on whether I have
>    * text=auto (a file was commited with LFs). At least in git 2.20.1

That sounds right. The default is that Git will never touch your line
endings (going into or out of the working tree). Turning on the
"text=auto" attribute for everything will tell it to start doing so
(according to core.eol, which on Windows is CRLF).

> 2) If the quote holds true then autocrlf=false is actually "true" (for
>    output conversion) on Windows by default because default core.eol
>    is 'native', which is CRLF. I believe autocrlf=false is supposed to
>    mean "no any conversion, input or output, read or write the repo as
>    is".

No, it only means "do not use the autocrlf feature". If you've
separately configured text attributes, it does not disable them.

-Peff

Re: unclear docs

[Sergey Lukashev]

Thank you. Does the paragraph about core.eol refers to the text attribute? It's written 'property' there. I was thinking it means whether git thinks file is text, using .gitattributes OR otherwise. Maybe changing this word will make it clearer?

28.01.2019, 19:09, "Jeff King" <peff@peff.net>:
> On Sun, Jan 27, 2019 at 02:55:23PM +0300, Sergey Lukashev wrote:
>
>>  Well, I have two problems:
>>  1) The endings I get with core.autocrlf=false depend on whether I have
>>     * text=auto (a file was commited with LFs). At least in git 2.20.1
>
> That sounds right. The default is that Git will never touch your line
> endings (going into or out of the working tree). Turning on the
> "text=auto" attribute for everything will tell it to start doing so
> (according to core.eol, which on Windows is CRLF).
>
>>  2) If the quote holds true then autocrlf=false is actually "true" (for
>>     output conversion) on Windows by default because default core.eol
>>     is 'native', which is CRLF. I believe autocrlf=false is supposed to
>>     mean "no any conversion, input or output, read or write the repo as
>>     is".
>
> No, it only means "do not use the autocrlf feature". If you've
> separately configured text attributes, it does not disable them.
>
> -Peff

Re: unclear docs

[Jeff King]

On Mon, Jan 28, 2019 at 09:12:00PM +0300, Sergey Lukashev wrote:

> Thank you. Does the paragraph about core.eol refers to the text
> attribute? It's written 'property' there. I was thinking it means
> whether git thinks file is text, using .gitattributes OR otherwise.
> Maybe changing this word will make it clearer?

Yeah, I think the word "property" is unnecessarily confusing. Here's
another patch (meant to go on top of the other).

-- >8 --
Subject: [PATCH] docs/config: clarify "text property" in core.eol

The word "property" is less clear than "attribute", which is used
elsewhere in the (surprise!) gitattributes documentation. So let's use
that.

While we're here, let's make the paragraph a little easier to read by
de-emphasizing the "when core.autocrlf is false" bit. Putting it in the
first sentence obscures the main content, and many readers won't care
about autocrlf (i.e., anyone who is just following the gitattributes(7)
advice, which mainly discusses "text" and "core.eol").

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/config/core.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index d0e6635fe0..69c73ffe30 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -121,11 +121,12 @@ core.quotePath::
 
 core.eol::
 	Sets the line ending type to use in the working directory for
-	files that have the `text` property set when core.autocrlf is false.
+	files that have the `text` attribute.
 	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.
+	conversion. Note that this value is ignored if `core.autocrlf`
+	is set to `true`.
 
 core.safecrlf::
 	If true, makes Git check if converting `CRLF` is reversible when
-- 
2.20.1.845.g57b86ca485

Re: unclear docs

[Torsten Bögershausen]

On Mon, Jan 28, 2019 at 01:31:17PM -0500, Jeff King wrote:
> On Mon, Jan 28, 2019 at 09:12:00PM +0300, Sergey Lukashev wrote:
>
> > Thank you. Does the paragraph about core.eol refers to the text
> > attribute? It's written 'property' there. I was thinking it means
> > whether git thinks file is text, using .gitattributes OR otherwise.
> > Maybe changing this word will make it clearer?
>
> Yeah, I think the word "property" is unnecessarily confusing. Here's
> another patch (meant to go on top of the other).

The property was originally meant to say:
- Either the "text" attribute is set in .gitattributes
  or
- The "text=auto" attribute is set in .gitattributes and
  Git auto-detects the file as text (and not as binary).



>
> -- >8 --
> Subject: [PATCH] docs/config: clarify "text property" in core.eol
>
> The word "property" is less clear than "attribute", which is used
> elsewhere in the (surprise!) gitattributes documentation. So let's use
> that.
>
> While we're here, let's make the paragraph a little easier to read by
> de-emphasizing the "when core.autocrlf is false" bit. Putting it in the
> first sentence obscures the main content, and many readers won't care
> about autocrlf (i.e., anyone who is just following the gitattributes(7)
> advice, which mainly discusses "text" and "core.eol").
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/config/core.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
> index d0e6635fe0..69c73ffe30 100644
> --- a/Documentation/config/core.txt
> +++ b/Documentation/config/core.txt
> @@ -121,11 +121,12 @@ core.quotePath::
>
>  core.eol::
>  	Sets the line ending type to use in the working directory for
> -	files that have the `text` property set when core.autocrlf is false.
> +	files that have the `text` attribute.
>  	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.
> +	conversion. Note that this value is ignored if `core.autocrlf`
> +	is set to `true`.

Should that be
 > +	conversion. Note that this value is ignored if `core.autocrlf`
 > +	is set to `true` or `input`.

As we look at convert.c:

static int text_eol_is_crlf(void)
{
  if (auto_crlf == AUTO_CRLF_TRUE)
    return 1;
  else if (auto_crlf == AUTO_CRLF_INPUT)
    return 0;
  if (core_eol == EOL_CRLF)
    return 1;
  if (core_eol == EOL_UNSET && EOL_NATIVE == EOL_CRLF)
    return 1;
  return 0;
}

Re: unclear docs

[Sergey Lukashev]

This is in fact how I got it but it seems to have changed since then.

29.01.2019, 10:38, "Torsten Bögershausen" <tboegi@web.de>:
>
> The property was originally meant to say:
> - Either the "text" attribute is set in .gitattributes
>   or
> - The "text=auto" attribute is set in .gitattributes and
>   Git auto-detects the file as text (and not as binary).
>

[PATCH v2 0/2] eol/autocrlf doc clarifications

[Jeff King]

On Tue, Jan 29, 2019 at 07:38:10AM +0000, Torsten Bögershausen wrote:

> > Yeah, I think the word "property" is unnecessarily confusing. Here's
> > another patch (meant to go on top of the other).
> 
> The property was originally meant to say:
> - Either the "text" attribute is set in .gitattributes
>   or
> - The "text=auto" attribute is set in .gitattributes and
>   Git auto-detects the file as text (and not as binary).

Thanks, that makes it much more clear what the original was trying to
say.

> > +	conversion. Note that this value is ignored if `core.autocrlf`
> > +	is set to `true`.
> 
> Should that be
>  > +	conversion. Note that this value is ignored if `core.autocrlf`
>  > +	is set to `true` or `input`.

Yes (and ditto in the other patch).

Here's a re-roll of both patches addressing these issues.

  [1/2]: doc/gitattributes: clarify "autocrlf overrides eol"
  [2/2]: docs/config: clarify "text property" in core.eol

 Documentation/config/core.txt   | 7 +++++--
 Documentation/gitattributes.txt | 4 +++-
 2 files changed, 8 insertions(+), 3 deletions(-)

-Peff

[PATCH v2 1/2] doc/gitattributes: clarify "autocrlf overrides eol"

[Jeff King]

We only override core.eol with core.autocrlf when the latter is set to
something besides "false".  Let's make this more clear, and point the
reader to the git-config definitions, which discuss this in more detail.

Noticed-by: Sergey Lukashev <lukashev.s@ya.ru>
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/gitattributes.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index b8392fc330..ce383e97dc 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -124,7 +124,9 @@ text file is normalized, its line endings are converted to LF in the
 repository.  To control what line ending style is used in the working
 directory, use the `eol` attribute for a single file and the
 `core.eol` configuration variable for all text files.
-Note that `core.autocrlf` overrides `core.eol`
+Note that setting `core.autocrlf` to `true` or `input` overrides
+`core.eol` (see the definitions of those options in
+linkgit:git-config[1]).
 
 Set::
 
-- 
2.20.1.845.g57b86ca485

[PATCH v2 2/2] docs/config: clarify "text property" in core.eol

[Jeff King]

The word "property" is vague here. Let's spell out that we mean the path
must be marked with the text attribute.

While we're here, let's make the paragraph a little easier to read by
de-emphasizing the "when core.autocrlf is false" bit. Putting it in the
first sentence obscures the main content, and many readers won't care
about autocrlf (i.e., anyone who is just following the gitattributes(7)
advice, which mainly discusses "text" and "core.eol").

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/config/core.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index d0e6635fe0..7e9b6c8f4c 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -121,11 +121,14 @@ core.quotePath::
 
 core.eol::
 	Sets the line ending type to use in the working directory for
-	files that have the `text` property set when core.autocrlf is false.
+	files that are marked as text (either by having the `text`
+	attribute set, or by having `text=auto` and Git auto-detecting
+	the contents as text).
 	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.
+	conversion. Note that this value is ignored if `core.autocrlf`
+	is set to `true` or `input`.
 
 core.safecrlf::
 	If true, makes Git check if converting `CRLF` is reversible when
-- 
2.20.1.845.g57b86ca485

Re: unclear docs

[Torsten Bögershausen]

On Sun, Jan 27, 2019 at 02:55:23PM +0300, Sergey Lukashev wrote:
> A follow up on my previous mail.
>
> Well, I have two problems:
> 1) The endings I get with core.autocrlf=false depend on whether I have * text=auto (a file was commited with LFs). At least in git 2.20.1
> 2) If the quote holds true then autocrlf=false is actually "true" (for output conversion) on Windows by default because default core.eol is 'native', which is CRLF. I believe autocrlf=false is supposed to mean "no any conversion, input or output, read or write the repo as is".
>
> 27.01.2019, 13:05, "Sergey Lukashev" <lukashev.s@ya.ru>:
> > Hello!
> >
> > Could somebody please explain what 'core.autocrlf overrides core.eol' means given that we have the following statement in core.eol docs:
> > "Sets the line ending type to use in the working directory for files that have the text property set when core.autocrlf is false".
> >
> > I would like to know which line endings I'm supposed to get in the working tree if I have core.autocrlf=false and '* text=auto' in .gitattributes on windows. I get CRLFs which is consistent with the last quote but I don't understand what 'core.autocrlf overrides core.eol' means in this case.
> >
> > Links:
> > https://git-scm.com/docs/git-config#git-config-coreeol
> > https://git-scm.com/docs/gitattributes#_checking_out_and_checking_in

Given your .gitattributes file, you will get:

core.autocrlf=false core.eol=CRLF    gives CRLF
core.autocrlf=false core.eol=LF      gives LF
core.autocrlf=false core.eol=native  gives CRLF under Windows, LF everywhere else
core.autocrlf=true                   gives CRLF (regardless of core.eol)

Does this help ?

Re: unclear docs

[Jeff King]

On Sun, Jan 27, 2019 at 12:58:58PM +0300, Sergey Lukashev wrote:

> Could somebody please explain what 'core.autocrlf overrides core.eol'
> means given that we have the following statement in core.eol docs:
> "Sets the line ending type to use in the working directory for files
> that have the text property set when core.autocrlf is false".
> 
> I would like to know which line endings I'm supposed to get in the
> working tree if I have core.autocrlf=false and '* text=auto' in
> .gitattributes on windows. I get CRLFs  which is consistent with the
> last quote but I don't understand what 'core.autocrlf overrides
> core.eol' means in this case.

I think it means:

  - if core.autocrlf is false (the default), then the text attribute and
    core.eol are used

  - if core.autocrlf is true, then that overrides the whole
    attribute/eol system

This unfortunate complexity is due to historical reasons (the
much-simpler autocrlf came first, and we had to maintain compatibility
with it). I agree the wording in the documentation is pretty vague.

Perhaps the patch below makes it more clear (I similarly looked at the
definitions of core.autocrlf and core.eol, but they make the distinction
that autocrlf only matters when "true").

-- >8 --
Subject: [PATCH] doc/gitattributes: clarify "autocrlf overrides eol"

We only override core.eol with core.autocrlf when the latter is set to
"true" (otherwise, core.eol would never do anything!). Let's make this
more clear, and point the reader to the git-config definitions, which
discuss this in more detail.

Noticed-by: Sergey Lukashev <lukashev.s@ya.ru>
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/gitattributes.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index b8392fc330..0ce8740e44 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -124,7 +124,8 @@ text file is normalized, its line endings are converted to LF in the
 repository.  To control what line ending style is used in the working
 directory, use the `eol` attribute for a single file and the
 `core.eol` configuration variable for all text files.
-Note that `core.autocrlf` overrides `core.eol`
+Note that setting `core.autocrlf` to `true` overrides `core.eol` (see
+the definitions of those options in linkgit:git-config[1]).
 
 Set::
 
-- 
2.20.1.845.g57b86ca485

Re: unclear docs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think it means:
>
>   - if core.autocrlf is false (the default), then the text attribute and
>     core.eol are used
>
>   - if core.autocrlf is true, then that overrides the whole
>     attribute/eol system

I think that matches my understanding.

> Subject: [PATCH] doc/gitattributes: clarify "autocrlf overrides eol"
>
> We only override core.eol with core.autocrlf when the latter is set to
> "true" (otherwise, core.eol would never do anything!). Let's make this

I fail to understand what "otherwise..." wants to say.  When
core.autocrlf is not "true", core.eol would not do anything?

But I can understand what the patch text says and it matches my
understanding of the overriding behaviour, so let's queue it.

Thanks.

> more clear, and point the reader to the git-config definitions, which
> discuss this in more detail.
>
> Noticed-by: Sergey Lukashev <lukashev.s@ya.ru>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/gitattributes.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index b8392fc330..0ce8740e44 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -124,7 +124,8 @@ text file is normalized, its line endings are converted to LF in the
>  repository.  To control what line ending style is used in the working
>  directory, use the `eol` attribute for a single file and the
>  `core.eol` configuration variable for all text files.
> -Note that `core.autocrlf` overrides `core.eol`
> +Note that setting `core.autocrlf` to `true` overrides `core.eol` (see
> +the definitions of those options in linkgit:git-config[1]).
>  
>  Set::

Re: unclear docs

[Jeff King]

On Mon, Jan 28, 2019 at 10:30:31AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think it means:
> >
> >   - if core.autocrlf is false (the default), then the text attribute and
> >     core.eol are used
> >
> >   - if core.autocrlf is true, then that overrides the whole
> >     attribute/eol system
> 
> I think that matches my understanding.
> 
> > Subject: [PATCH] doc/gitattributes: clarify "autocrlf overrides eol"
> >
> > We only override core.eol with core.autocrlf when the latter is set to
> > "true" (otherwise, core.eol would never do anything!). Let's make this
> 
> I fail to understand what "otherwise..." wants to say.  When
> core.autocrlf is not "true", core.eol would not do anything?

I meant that if core.autocrlf overrides core.eol also when it is not set
to true, then there would be no way to use core.eol, because the former
must be either true or false.

I suppose one could argue that it can be "unset", though.

> But I can understand what the patch text says and it matches my
> understanding of the overriding behaviour, so let's queue it.

Thanks. I just posted a follow-up patch elsewhere in the thread. I think
it makes sense to do both.

-Peff

Re: unclear docs

[Sergey Lukashev]

autocrlf can be "input" as well.

28.01.2019, 21:32, "Jeff King" <peff@peff.net>:
> On Mon, Jan 28, 2019 at 10:30:31AM -0800, Junio C Hamano wrote:
>
>>  Jeff King <peff@peff.net> writes:
>>
>>  > I think it means:
>>  >
>>  > - if core.autocrlf is false (the default), then the text attribute and
>>  > core.eol are used
>>  >
>>  > - if core.autocrlf is true, then that overrides the whole
>>  > attribute/eol system
>>
>>  I think that matches my understanding.
>>
>>  > Subject: [PATCH] doc/gitattributes: clarify "autocrlf overrides eol"
>>  >
>>  > We only override core.eol with core.autocrlf when the latter is set to
>>  > "true" (otherwise, core.eol would never do anything!). Let's make this
>>
>>  I fail to understand what "otherwise..." wants to say. When
>>  core.autocrlf is not "true", core.eol would not do anything?
>
> I meant that if core.autocrlf overrides core.eol also when it is not set
> to true, then there would be no way to use core.eol, because the former
> must be either true or false.
>

Re: unclear docs

[Jeff King]

On Mon, Jan 28, 2019 at 11:16:54PM +0300, Sergey Lukashev wrote:

> >>  > Subject: [PATCH] doc/gitattributes: clarify "autocrlf overrides eol"
> >>  >
> >>  > We only override core.eol with core.autocrlf when the latter is set to
> >>  > "true" (otherwise, core.eol would never do anything!). Let's make this
> >>
> >>  I fail to understand what "otherwise..." wants to say. When
> >>  core.autocrlf is not "true", core.eol would not do anything?
> >
> > I meant that if core.autocrlf overrides core.eol also when it is not set
> > to true, then there would be no way to use core.eol, because the former
> > must be either true or false.
>
> autocrlf can be "input" as well.

Ah, true. Should the documentation actually say "if core.autocrlf is not
'false'" then? I _think_ so from my reading of the code, but it would be
nice to have confirmation from somebody better versed in the line-ending
code than I am (+cc Torsten).

-Peff