git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] Improvements to tests and docs for .gitattributes eol
@ 2022-01-11  2:15 brian m. carlson
  2022-01-11  2:15 ` [PATCH 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: brian m. carlson @ 2022-01-11  2:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Torsten Bögershausen

I was answering a question on StackOverflow recently about the
interaction between text=auto and eol, and someone pointed out to me
that what I had written, which was based on the documentation, was not
correct as of Git 2.10 (and more specifically 6523728499 ("convert:
unify the "auto" handling of CRLF", 2016-06-28)).

When I set out to document the behavior correctly, I ran into the fact
that the tests, where I looked for examples of how this behaves, didn't
have any tests for some of these cases, and so I had some trouble
documenting this clearly and accurately.  So this series basically just
adds some tests for existing behavior so we don't change it (and so I
could figure out how it works) and then updates the documentation
accordingly.

I tried to make the docs as specific as possible, since I needed them to
be specific and accurate here, and I felt like speaking affirmatively
about the behavior would be clearer than speaking negatively about the
behavior (I tried both).  I would of course be delighted to hear
suggestions on how this could be clearer or easier to understand.

I realize that 2.35.0-rc0 has just come out and so this won't be picked
up right away, which is fine, but I thought I'd send it out
nevertheless (mostly so I don't forget).

brian m. carlson (2):
  t0027: add tests for eol without text in .gitattributes
  docs: correct documentation about eol attribute

 Documentation/gitattributes.txt | 11 ++++++-----
 t/t0027-auto-crlf.sh            |  6 ++++++
 2 files changed, 12 insertions(+), 5 deletions(-)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] t0027: add tests for eol without text in .gitattributes
  2022-01-11  2:15 [PATCH 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
@ 2022-01-11  2:15 ` brian m. carlson
  2022-01-11  2:15 ` [PATCH 2/2] docs: correct documentation about eol attribute brian m. carlson
  2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
  2 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2022-01-11  2:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Torsten Bögershausen

Right now, it isn't clear what the behavior is when the eol attribute is
set in .gitattributes but the text attribute is not.  Let's add some
tests to document this behavior in our code, which happens to be that
the behavior is as if we set the text attribute implicitly.  This will
make sure we don't accidentally change the behavior, which somebody is
probably relying on, and serve as documentation to developers.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0027-auto-crlf.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 4a5c5c602c..c5f7ac63b0 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -597,6 +597,12 @@ do
 	# auto: core.autocrlf=false and core.eol unset(or native) uses native eol
 	checkout_files     auto  "$id" ""     false   ""       $NL   CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
 	checkout_files     auto  "$id" ""     false   native   $NL   CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
+	# core.autocrlf false, .gitattributes sets eol
+	checkout_files     ""    "$id" "lf"   false   ""       LF    CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
+	checkout_files     ""    "$id" "crlf" false   ""       CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul
+	# core.autocrlf true, .gitattributes sets eol
+	checkout_files     ""    "$id" "lf"   true    ""       LF    CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
+	checkout_files     ""    "$id" "crlf" true    ""       CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul
 done
 
 # The rest of the tests are unique; do the usual linting.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/2] docs: correct documentation about eol attribute
  2022-01-11  2:15 [PATCH 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
  2022-01-11  2:15 ` [PATCH 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
@ 2022-01-11  2:15 ` brian m. carlson
  2022-01-11 18:30   ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
  2 siblings, 1 reply; 19+ messages in thread
From: brian m. carlson @ 2022-01-11  2:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Torsten Bögershausen

The documentation for the eol attribute states that it is "effectively
setting the text attribute".  However, this implies that it forces the
text attribute to always be set, which has not been the case since
6523728499 ("convert: unify the "auto" handling of CRLF", 2016-06-28).
Let's avoid confusing users (and the present author when trying to
describe Git's behavior to others) by clearly documenting in which
cases the "eol" attribute has effect.

Specifically, the attribute always has an effect unless the file is
explicitly set as -text, or the file is set as text=auto and the file is
detected as binary.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/gitattributes.txt | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 83fd4e19a4..60984a4682 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -160,11 +160,12 @@ unspecified.
 ^^^^^
 
 This attribute sets a specific line-ending style to be used in the
-working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.  Note that
-setting this attribute on paths which are in the index with CRLF line
-endings may make the paths to be considered dirty.  Adding the path to
-the index again will normalize the line endings in the index.
+working directory.  This attribute has effect only if the `text`
+attribute is set or unspecified, or if it is set to `auto` and the file
+is detected as text.  Note that setting this attribute on paths which
+are in the index with CRLF line endings may make the paths to be
+considered dirty. Adding the path to the index again will normalize the
+line endings in the index.
 
 Set to string value "crlf"::
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] docs: correct documentation about eol attribute
  2022-01-11  2:15 ` [PATCH 2/2] docs: correct documentation about eol attribute brian m. carlson
@ 2022-01-11 18:30   ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  2022-01-11 22:40     ` brian m. carlson
  0 siblings, 1 reply; 19+ messages in thread
From: Torsten =?unknown-8bit?Q?B=C3=B6gershausen?= @ 2022-01-11 18:30 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Junio C Hamano

Hej Brian,
thanks for digging into this.

Could you be so kind to send the stackoverflow issue ?
(You can send it to me only)

I have some comments/questions, out of my head.

On Tue, Jan 11, 2022 at 02:15:07AM +0000, brian m. carlson wrote:
> The documentation for the eol attribute states that it is "effectively
> setting the text attribute".
> Let's avoid confusing users (and the present author when trying to
> describe Git's behavior to others) by clearly documenting in which
> cases the "eol" attribute has effect.
>
> Specifically, the attribute always has an effect unless the file is
> explicitly set as -text, or the file is set as text=auto and the file is
> detected as binary.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  Documentation/gitattributes.txt | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 83fd4e19a4..60984a4682 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -160,11 +160,12 @@ unspecified.
>  ^^^^^
>
>  This attribute sets a specific line-ending style to be used in the
> -working directory.  It enables end-of-line conversion without any
> -content checks, effectively setting the `text` attribute.  Note that
> -setting this attribute on paths which are in the index with CRLF line
> -endings may make the paths to be considered dirty.  Adding the path to
> -the index again will normalize the line endings in the index.
> +working directory.  This attribute has effect only if the `text`
> +attribute is set or unspecified, or if it is set to `auto` and the file
> +is detected as text.



>  Note that setting this attribute on paths which
> +are in the index with CRLF line endings may make the paths to be
> +considered dirty. Adding the path to the index again will normalize the
> +line endings in the index.

I think that this can be loosened as well. And, beside this, the "dirty"
warning about setting attributes could be written as part of the "text"
attribute as well. I dunno. Here is a possible suggestion:


  Note that setting this attribute on paths which are in the index with CRLF
  line endings may make the paths to be considered dirty - unless "text=auto"
  is set. `git ls-files --eol` can be used to check the "line ending status".
  Adding the path to the index again will normalize the line endings in the index.

>
>  Set to string value "crlf"::
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] docs: correct documentation about eol attribute
  2022-01-11 18:30   ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
@ 2022-01-11 22:40     ` brian m. carlson
  2022-01-12 15:16       ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  0 siblings, 1 reply; 19+ messages in thread
From: brian m. carlson @ 2022-01-11 22:40 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

On 2022-01-11 at 18:30:03, Torsten Bögershausen wrote:
> Hej Brian,
> thanks for digging into this.
> 
> Could you be so kind to send the stackoverflow issue ?
> (You can send it to me only)

I'll just post it here publicly, since I think there's value to folks
seeing what questions users have:

https://stackoverflow.com/questions/70633469/what-is-the-difference-between-text-auto-and-text-auto-eol-lf/70636508?

> On Tue, Jan 11, 2022 at 02:15:07AM +0000, brian m. carlson wrote:
> >  Note that setting this attribute on paths which
> > +are in the index with CRLF line endings may make the paths to be
> > +considered dirty. Adding the path to the index again will normalize the
> > +line endings in the index.
> 
> I think that this can be loosened as well. And, beside this, the "dirty"
> warning about setting attributes could be written as part of the "text"
> attribute as well. I dunno. Here is a possible suggestion:
> 
> 
>   Note that setting this attribute on paths which are in the index with CRLF
>   line endings may make the paths to be considered dirty - unless "text=auto"
>   is set. `git ls-files --eol` can be used to check the "line ending status".
>   Adding the path to the index again will normalize the line endings in the index.

I'm not sure that's correct, though.  The problem is if the file is
detected as text, which it might well be if text=auto is set.  Or am I
not understanding something correctly?
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] docs: correct documentation about eol attribute
  2022-01-11 22:40     ` brian m. carlson
@ 2022-01-12 15:16       ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  0 siblings, 0 replies; 19+ messages in thread
From: Torsten =?unknown-8bit?Q?B=C3=B6gershausen?= @ 2022-01-12 15:16 UTC (permalink / raw)
  To: brian m. carlson, git, Junio C Hamano

On Tue, Jan 11, 2022 at 10:40:47PM +0000, brian m. carlson wrote:
> On 2022-01-11 at 18:30:03, Torsten B??gershausen wrote:
> > Hej Brian,
> > thanks for digging into this.
> >
> > Could you be so kind to send the stackoverflow issue ?
> > (You can send it to me only)
>
> I'll just post it here publicly, since I think there's value to folks
> seeing what questions users have:

Thanks - Please see the comments inline, as usual.

>
> https://stackoverflow.com/questions/70633469/what-is-the-difference-between-text-auto-and-text-auto-eol-lf/70636508?

To pick up the question here:

  I was reading about the .gitattributes file and the rule to force line endings
  in some tutorials it's written like
  * text=auto
  and in some others, it's like
  * text=auto eol=lf
  at the first line of the file.

  Are there any differences? what does the first one exactly do? Does it even force any line endings?
[]

Yes, there are differences.
The line
* text=auto
will make sure that all by-Git-as-text-files-detected files
will be commited with LF into the repo.
CRLF in the working tree will become LF in the repo.

When the files are checkout, the line endings depend on local
git config settings:
core.autocrlf=true will give CRLF
core.autocrlf=input will give LF
When core.autocrlf is false (or unset) git looks at core.eol:
core.eol=crlf gives CRLF
core.eol=lf gives LF
core.eol unset (or native) will use the the native line endings,
CRLF on Windows, LF everywhere else.
--------------
Let's look at
* text=auto eol=lf

Here Git does not look at any local config variables.
All files will be checkout out with LF, even on Windows.

>
> > On Tue, Jan 11, 2022 at 02:15:07AM +0000, brian m. carlson wrote:
> > >  Note that setting this attribute on paths which
> > > +are in the index with CRLF line endings may make the paths to be
> > > +considered dirty. Adding the path to the index again will normalize the
> > > +line endings in the index.
> >
> > I think that this can be loosened as well. And, beside this, the "dirty"
> > warning about setting attributes could be written as part of the "text"
> > attribute as well. I dunno. Here is a possible suggestion:
> >
> >
> >   Note that setting this attribute on paths which are in the index with CRLF
> >   line endings may make the paths to be considered dirty - unless "text=auto"
> >   is set. `git ls-files --eol` can be used to check the "line ending status".
> >   Adding the path to the index again will normalize the line endings in the index.
>
> I'm not sure that's correct, though.  The problem is if the file is
> detected as text, which it might well be if text=auto is set.  Or am I
> not understanding something correctly?

Which problem are we talking about ?
Files that once had been commited with CRLF into the repo are
now considered dirty?
The "new safer autocrlf-handling" will not try to normalize them
when text=auto is specified.
They keep their existing line endings at checkout or checkin.

I hope this makes sense ?


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-01-11  2:15 [PATCH 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
  2022-01-11  2:15 ` [PATCH 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
  2022-01-11  2:15 ` [PATCH 2/2] docs: correct documentation about eol attribute brian m. carlson
@ 2022-02-14  2:08 ` brian m. carlson
  2022-02-14  2:08   ` [PATCH v2 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
                     ` (3 more replies)
  2 siblings, 4 replies; 19+ messages in thread
From: brian m. carlson @ 2022-02-14  2:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Torsten Bögershausen

I was answering a question on StackOverflow recently about the
interaction between text=auto and eol, and someone pointed out to me
that what I had written, which was based on the documentation, was not
correct as of Git 2.10 (and more specifically 6523728499 ("convert:
unify the "auto" handling of CRLF", 2016-06-28)).

When I set out to document the behavior correctly, I ran into the fact
that the tests, where I looked for examples of how this behaves, didn't
have any tests for some of these cases, and so I had some trouble
documenting this clearly and accurately.  So this series basically just
adds some tests for existing behavior so we don't change it (and so I
could figure out how it works) and then updates the documentation
accordingly.

Changes from v1:
* Correct documentation inaccuracy with respect to text=auto.

brian m. carlson (2):
  t0027: add tests for eol without text in .gitattributes
  docs: correct documentation about eol attribute

 Documentation/gitattributes.txt | 12 +++++++-----
 t/t0027-auto-crlf.sh            |  6 ++++++
 2 files changed, 13 insertions(+), 5 deletions(-)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/2] t0027: add tests for eol without text in .gitattributes
  2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
@ 2022-02-14  2:08   ` brian m. carlson
  2022-02-14  2:08   ` [PATCH v2 2/2] docs: correct documentation about eol attribute brian m. carlson
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2022-02-14  2:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Torsten Bögershausen

Right now, it isn't clear what the behavior is when the eol attribute is
set in .gitattributes but the text attribute is not.  Let's add some
tests to document this behavior in our code, which happens to be that
the behavior is as if we set the text attribute implicitly.  This will
make sure we don't accidentally change the behavior, which somebody is
probably relying on, and serve as documentation to developers.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0027-auto-crlf.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 4a5c5c602c..c5f7ac63b0 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -597,6 +597,12 @@ do
 	# auto: core.autocrlf=false and core.eol unset(or native) uses native eol
 	checkout_files     auto  "$id" ""     false   ""       $NL   CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
 	checkout_files     auto  "$id" ""     false   native   $NL   CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
+	# core.autocrlf false, .gitattributes sets eol
+	checkout_files     ""    "$id" "lf"   false   ""       LF    CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
+	checkout_files     ""    "$id" "crlf" false   ""       CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul
+	# core.autocrlf true, .gitattributes sets eol
+	checkout_files     ""    "$id" "lf"   true    ""       LF    CRLF  CRLF_mix_LF  LF_mix_CR    LF_nul
+	checkout_files     ""    "$id" "crlf" true    ""       CRLF  CRLF  CRLF         CRLF_mix_CR  CRLF_nul
 done
 
 # The rest of the tests are unique; do the usual linting.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 2/2] docs: correct documentation about eol attribute
  2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
  2022-02-14  2:08   ` [PATCH v2 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
@ 2022-02-14  2:08   ` brian m. carlson
  2022-02-14 14:52   ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol Derrick Stolee
  2022-02-14 18:15   ` Junio C Hamano
  3 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2022-02-14  2:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Torsten Bögershausen

The documentation for the eol attribute states that it is "effectively
setting the text attribute".  However, this implies that it forces the
text attribute to always be set, which has not been the case since
6523728499 ("convert: unify the "auto" handling of CRLF", 2016-06-28).
Let's avoid confusing users (and the present author when trying to
describe Git's behavior to others) by clearly documenting in which
cases the "eol" attribute has effect.

Specifically, the attribute always has an effect unless the file is
explicitly set as -text, or the file is set as text=auto and the file is
detected as binary or has CRLF endings.  It used to be the case that
text=auto did cause automatic conversion of files with CRLF endings, but
that is no longer the case, so let's document that fact as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/gitattributes.txt | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 83fd4e19a4..a71dad2674 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -160,11 +160,13 @@ unspecified.
 ^^^^^
 
 This attribute sets a specific line-ending style to be used in the
-working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.  Note that
-setting this attribute on paths which are in the index with CRLF line
-endings may make the paths to be considered dirty.  Adding the path to
-the index again will normalize the line endings in the index.
+working directory.  This attribute has effect only if the `text`
+attribute is set or unspecified, or if it is set to `auto`, the file is
+detected as text, and it is stored with LF endings in the index.  Note
+that setting this attribute on paths which are in the index with CRLF
+line endings may make the paths to be considered dirty unless
+`text=auto` is set. Adding the path to the index again will normalize
+the line endings in the index.
 
 Set to string value "crlf"::
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
  2022-02-14  2:08   ` [PATCH v2 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
  2022-02-14  2:08   ` [PATCH v2 2/2] docs: correct documentation about eol attribute brian m. carlson
@ 2022-02-14 14:52   ` Derrick Stolee
  2022-02-14 18:15   ` Junio C Hamano
  3 siblings, 0 replies; 19+ messages in thread
From: Derrick Stolee @ 2022-02-14 14:52 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Junio C Hamano, Torsten Bögershausen

On 2/13/2022 9:08 PM, brian m. carlson wrote:
> I was answering a question on StackOverflow recently about the
> interaction between text=auto and eol, and someone pointed out to me
> that what I had written, which was based on the documentation, was not
> correct as of Git 2.10 (and more specifically 6523728499 ("convert:
> unify the "auto" handling of CRLF", 2016-06-28)).
> 
> When I set out to document the behavior correctly, I ran into the fact
> that the tests, where I looked for examples of how this behaves, didn't
> have any tests for some of these cases, and so I had some trouble
> documenting this clearly and accurately.  So this series basically just
> adds some tests for existing behavior so we don't change it (and so I
> could figure out how it works) and then updates the documentation
> accordingly.

Thanks for these updates. They look good as-is.

-Stolee


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
                     ` (2 preceding siblings ...)
  2022-02-14 14:52   ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol Derrick Stolee
@ 2022-02-14 18:15   ` Junio C Hamano
  2022-02-14 20:46     ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  3 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2022-02-14 18:15 UTC (permalink / raw)
  To: brian m. carlson; +Cc: git, Torsten Bögershausen

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I was answering a question on StackOverflow recently about the
> interaction between text=auto and eol, and someone pointed out to me
> that what I had written, which was based on the documentation, was not
> correct as of Git 2.10 (and more specifically 6523728499 ("convert:
> unify the "auto" handling of CRLF", 2016-06-28)).
>
> When I set out to document the behavior correctly, I ran into the fact
> that the tests, where I looked for examples of how this behaves, didn't
> have any tests for some of these cases, and so I had some trouble
> documenting this clearly and accurately.  So this series basically just
> adds some tests for existing behavior so we don't change it (and so I
> could figure out how it works) and then updates the documentation
> accordingly.

This seems to be a replacement of the two-patch series that was
merged to 'master' at 8db2f665 (Merge branch 'bc/clarify-eol-attr',
2022-02-11) and was merged to 'next' at dc1db4bd (Merge branch
'bc/clarify-eol-attr' into next, 2022-02-04).  The changes seem to
be in the second step to update Documentation/gitattributes.txt and
it needs to be made an incremental update.

Thanks.

---- >8 -----
From: brian m. carlson <sandals@crustytoothpaste.net>
Subject: doc: clarify interaction between 'eol' and text=auto

The `eol` takes effect on text files only when the index has the
contents in LF line endings.  Paths with contents in CRLF line
endings in the index may become dirty unless text=auto.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/gitattributes.txt | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git c/Documentation/gitattributes.txt w/Documentation/gitattributes.txt
index 60984a4682..a71dad2674 100644
--- c/Documentation/gitattributes.txt
+++ w/Documentation/gitattributes.txt
@@ -161,11 +161,12 @@ unspecified.
 
 This attribute sets a specific line-ending style to be used in the
 working directory.  This attribute has effect only if the `text`
-attribute is set or unspecified, or if it is set to `auto` and the file
-is detected as text.  Note that setting this attribute on paths which
-are in the index with CRLF line endings may make the paths to be
-considered dirty. Adding the path to the index again will normalize the
-line endings in the index.
+attribute is set or unspecified, or if it is set to `auto`, the file is
+detected as text, and it is stored with LF endings in the index.  Note
+that setting this attribute on paths which are in the index with CRLF
+line endings may make the paths to be considered dirty unless
+`text=auto` is set. Adding the path to the index again will normalize
+the line endings in the index.
 
 Set to string value "crlf"::
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-14 18:15   ` Junio C Hamano
@ 2022-02-14 20:46     ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  2022-02-15  0:15       ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Torsten =?unknown-8bit?Q?B=C3=B6gershausen?= @ 2022-02-14 20:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git

[]
> This seems to be a replacement of the two-patch series that was
> merged to 'master' at 8db2f665 (Merge branch 'bc/clarify-eol-attr',
> 2022-02-11) and was merged to 'next' at dc1db4bd (Merge branch
> 'bc/clarify-eol-attr' into next, 2022-02-04).  The changes seem to
> be in the second step to update Documentation/gitattributes.txt and
> it needs to be made an incremental update.
>
> Thanks.
>
> ---- >8 -----
> From: brian m. carlson <sandals@crustytoothpaste.net>
> Subject: doc: clarify interaction between 'eol' and text=auto
>
> The `eol` takes effect on text files only when the index has the
> contents in LF line endings.  Paths with contents in CRLF line
> endings in the index may become dirty unless text=auto.

That is a nice, precise and short summary here in the commit message
as well as the patch further down.

>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/gitattributes.txt | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git c/Documentation/gitattributes.txt w/Documentation/gitattributes.txt
> index 60984a4682..a71dad2674 100644
> --- c/Documentation/gitattributes.txt
> +++ w/Documentation/gitattributes.txt
> @@ -161,11 +161,12 @@ unspecified.
>
>  This attribute sets a specific line-ending style to be used in the
>  working directory.  This attribute has effect only if the `text`
> -attribute is set or unspecified, or if it is set to `auto` and the file
> -is detected as text.  Note that setting this attribute on paths which
> -are in the index with CRLF line endings may make the paths to be
> -considered dirty. Adding the path to the index again will normalize the
> -line endings in the index.
> +attribute is set or unspecified, or if it is set to `auto`, the file is
> +detected as text, and it is stored with LF endings in the index.  Note
> +that setting this attribute on paths which are in the index with CRLF
> +line endings may make the paths to be considered dirty unless
> +`text=auto` is set. Adding the path to the index again will normalize
> +the line endings in the index.
>
>  Set to string value "crlf"::
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-14 20:46     ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
@ 2022-02-15  0:15       ` Junio C Hamano
  2022-02-15  7:05         ` Johannes Sixt
  0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2022-02-15  0:15 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: brian m. carlson, git

Torsten Bögershausen <tboegi@web.de> writes:

>> ---- >8 -----
>> From: brian m. carlson <sandals@crustytoothpaste.net>
>> Subject: doc: clarify interaction between 'eol' and text=auto
>>
>> The `eol` takes effect on text files only when the index has the
>> contents in LF line endings.  Paths with contents in CRLF line
>> endings in the index may become dirty unless text=auto.
>
> That is a nice, precise and short summary here in the commit message
> as well as the patch further down.

Thanks.  Then let's queue it for 'next' and merge it down.

>>
>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  Documentation/gitattributes.txt | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git c/Documentation/gitattributes.txt w/Documentation/gitattributes.txt
>> index 60984a4682..a71dad2674 100644
>> --- c/Documentation/gitattributes.txt
>> +++ w/Documentation/gitattributes.txt
>> @@ -161,11 +161,12 @@ unspecified.
>>
>>  This attribute sets a specific line-ending style to be used in the
>>  working directory.  This attribute has effect only if the `text`
>> -attribute is set or unspecified, or if it is set to `auto` and the file
>> -is detected as text.  Note that setting this attribute on paths which
>> -are in the index with CRLF line endings may make the paths to be
>> -considered dirty. Adding the path to the index again will normalize the
>> -line endings in the index.
>> +attribute is set or unspecified, or if it is set to `auto`, the file is
>> +detected as text, and it is stored with LF endings in the index.  Note
>> +that setting this attribute on paths which are in the index with CRLF
>> +line endings may make the paths to be considered dirty unless
>> +`text=auto` is set. Adding the path to the index again will normalize
>> +the line endings in the index.
>>
>>  Set to string value "crlf"::
>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-15  0:15       ` Junio C Hamano
@ 2022-02-15  7:05         ` Johannes Sixt
  2022-02-15 22:46           ` brian m. carlson
  0 siblings, 1 reply; 19+ messages in thread
From: Johannes Sixt @ 2022-02-15  7:05 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen; +Cc: brian m. carlson, git

Am 15.02.22 um 01:15 schrieb Junio C Hamano:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>>> ---- >8 -----
>>> From: brian m. carlson <sandals@crustytoothpaste.net>
>>> Subject: doc: clarify interaction between 'eol' and text=auto
>>>
>>> The `eol` takes effect on text files only when the index has the
>>> contents in LF line endings.  Paths with contents in CRLF line
>>> endings in the index may become dirty unless text=auto.
>>
>> That is a nice, precise and short summary here in the commit message
>> as well as the patch further down.
> 
> Thanks.  Then let's queue it for 'next' and merge it down.
> 
>>>
>>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>> ---
>>>  Documentation/gitattributes.txt | 11 ++++++-----
>>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git c/Documentation/gitattributes.txt w/Documentation/gitattributes.txt
>>> index 60984a4682..a71dad2674 100644
>>> --- c/Documentation/gitattributes.txt
>>> +++ w/Documentation/gitattributes.txt
>>> @@ -161,11 +161,12 @@ unspecified.
>>>
>>>  This attribute sets a specific line-ending style to be used in the
>>>  working directory.  This attribute has effect only if the `text`
>>> -attribute is set or unspecified, or if it is set to `auto` and the file
>>> -is detected as text.  Note that setting this attribute on paths which
>>> -are in the index with CRLF line endings may make the paths to be
>>> -considered dirty. Adding the path to the index again will normalize the
>>> -line endings in the index.
>>> +attribute is set or unspecified, or if it is set to `auto`, the file is
>>> +detected as text, and it is stored with LF endings in the index.  Note
>>> +that setting this attribute on paths which are in the index with CRLF
>>> +line endings may make the paths to be considered dirty unless
>>> +`text=auto` is set. Adding the path to the index again will normalize
>>> +the line endings in the index.

Sorry, I don't find this description clear at all due to the many 'or's
and 'and's and no indication which parts belong together. The original
text was clear (but, of course, not helpful if it was wrong).

I suggest to rewrite the paragraph into format with bullet points:

   ... only if one of the following is true:

  - is set and foo or bar
  - is unspecified and either
      - this
      - or that
  - is set to auto but not...

or something along the lines. I can't propose actual text because I have
no clue what the truth is.

-- Hannes

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-15  7:05         ` Johannes Sixt
@ 2022-02-15 22:46           ` brian m. carlson
  2022-02-16  7:00             ` Johannes Sixt
  0 siblings, 1 reply; 19+ messages in thread
From: brian m. carlson @ 2022-02-15 22:46 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Torsten Bögershausen, git

[-- Attachment #1: Type: text/plain, Size: 817 bytes --]

On 2022-02-15 at 07:05:44, Johannes Sixt wrote:
> Sorry, I don't find this description clear at all due to the many 'or's
> and 'and's and no indication which parts belong together. The original
> text was clear (but, of course, not helpful if it was wrong).
> 
> I suggest to rewrite the paragraph into format with bullet points:
> 
>    ... only if one of the following is true:
> 
>   - is set and foo or bar
>   - is unspecified and either
>       - this
>       - or that
>   - is set to auto but not...
> 
> or something along the lines. I can't propose actual text because I have
> no clue what the truth is.

Unfortunately, the fact is that this behaviour is complicated.  I can
try a reroll with a bulleted list, though.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-15 22:46           ` brian m. carlson
@ 2022-02-16  7:00             ` Johannes Sixt
  2022-02-16 10:28               ` brian m. carlson
  0 siblings, 1 reply; 19+ messages in thread
From: Johannes Sixt @ 2022-02-16  7:00 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Torsten Bögershausen, git, Junio C Hamano

Am 15.02.22 um 23:46 schrieb brian m. carlson:
> On 2022-02-15 at 07:05:44, Johannes Sixt wrote:
>> Sorry, I don't find this description clear at all due to the many 'or's
>> and 'and's and no indication which parts belong together. The original
>> text was clear (but, of course, not helpful if it was wrong).
>>
>> I suggest to rewrite the paragraph into format with bullet points:
>>
>>    ... only if one of the following is true:
>>
>>   - is set and foo or bar
>>   - is unspecified and either
>>       - this
>>       - or that
>>   - is set to auto but not...
>>
>> or something along the lines. I can't propose actual text because I have
>> no clue what the truth is.
> 
> Unfortunately, the fact is that this behaviour is complicated.  I can
> try a reroll with a bulleted list, though.

Just so you know where my confusion arises from: Your updated text has
the structure (as I read it)

   if ... set or unspecified or if auto then ... detected ... and LF

It is unclear whether the 'then' conditions apply only to 'if auto'.
Even if the additional 'if' in the middle makes me think that the
'then's apply only to the 'auto' case, it is sufficently vage because in
my mental model there is not much difference between an 'unset' and a
set-to-'auto' attribute, and I wonder why the 'then's should not apply
to the 'unset' case as well.

Moreover, after re-reading the text, I notice that text may be read as
"this attribute has an effect only if <conditions>" where <conditions>
basically means "always except for when the 'if auto' case is not met",
right? Would it perhaps be better to write "has no effect if <very
specific condition>"?

-- Hannes

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-16  7:00             ` Johannes Sixt
@ 2022-02-16 10:28               ` brian m. carlson
  2022-02-16 11:52                 ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  2022-02-16 19:02                 ` Johannes Sixt
  0 siblings, 2 replies; 19+ messages in thread
From: brian m. carlson @ 2022-02-16 10:28 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Torsten Bögershausen, git, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]

On 2022-02-16 at 07:00:24, Johannes Sixt wrote:
> Just so you know where my confusion arises from: Your updated text has
> the structure (as I read it)
> 
>    if ... set or unspecified or if auto then ... detected ... and LF
> 
> It is unclear whether the 'then' conditions apply only to 'if auto'.
> Even if the additional 'if' in the middle makes me think that the
> 'then's apply only to the 'auto' case, it is sufficently vage because in
> my mental model there is not much difference between an 'unset' and a
> set-to-'auto' attribute, and I wonder why the 'then's should not apply
> to the 'unset' case as well.
> 
> Moreover, after re-reading the text, I notice that text may be read as
> "this attribute has an effect only if <conditions>" where <conditions>
> basically means "always except for when the 'if auto' case is not met",
> right? Would it perhaps be better to write "has no effect if <very
> specific condition>"?

The situation is that eol is in effect if and only if:

* text is set;
* text is unspecified; or
* text is auto, the file is detected as text, and the file has LF line
  endings in the index.

Alternately, it has no effect if and only if:

* text is unset;
* text is auto and the file is detected as binary; or
* text is auto and the file is detected as text and has CRLF line
  endings.

I'm not sure one reads significantly easier than the other.  I slightly
prefer the former because it has fewer conditions with multiple nested
entries, though.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-16 10:28               ` brian m. carlson
@ 2022-02-16 11:52                 ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
  2022-02-16 19:02                 ` Johannes Sixt
  1 sibling, 0 replies; 19+ messages in thread
From: Torsten =?unknown-8bit?Q?B=C3=B6gershausen?= @ 2022-02-16 11:52 UTC (permalink / raw)
  To: brian m. carlson, Johannes Sixt, git, Junio C Hamano

On Wed, Feb 16, 2022 at 10:28:24AM +0000, brian m. carlson wrote:
> On 2022-02-16 at 07:00:24, Johannes Sixt wrote:
> > Just so you know where my confusion arises from: Your updated text has
> > the structure (as I read it)
> >
> >    if ... set or unspecified or if auto then ... detected ... and LF
> >
> > It is unclear whether the 'then' conditions apply only to 'if auto'.
> > Even if the additional 'if' in the middle makes me think that the
> > 'then's apply only to the 'auto' case, it is sufficently vage because in
> > my mental model there is not much difference between an 'unset' and a
> > set-to-'auto' attribute, and I wonder why the 'then's should not apply
> > to the 'unset' case as well.
> >
> > Moreover, after re-reading the text, I notice that text may be read as
> > "this attribute has an effect only if <conditions>" where <conditions>
> > basically means "always except for when the 'if auto' case is not met",
> > right? Would it perhaps be better to write "has no effect if <very
> > specific condition>"?
>
> The situation is that eol is in effect if and only if:

Well written

>
> * text is set;
> * text is unspecified; or
> * text is auto, the file is detected as text, and the file has LF line
>   endings in the index.
>
> Alternately, it has no effect if and only if:
>
> * text is unset;
> * text is auto and the file is detected as binary; or
> * text is auto and the file is detected as text and has CRLF line
>   endings.
... CRLF line endings in the index.
                      ^^^^^^^^^^^^

One of the reasons that the eol attribute is not 100% well-specified
is that people should use the eol attribute together with text.

Either
text=auto eol=crlf
or
text=auto eol=lf
or
text eol=crlf
or
text eol=lf

Older git versions did treat
* text=auto
* eol=crlf

The same as
* text eol=crlf
Which did corrupt binary files.

Never git versions treat
* text=auto
* eol=crlf
as
* text=auto eol=crlf
in the sense that only auto-detected text files are converted,
if they had not been commited with crlf before.

In that sense I feel that the short form
eol=crlf
should be avoided

>
> I'm not sure one reads significantly easier than the other.  I slightly
> prefer the former because it has fewer conditions with multiple nested
> entries, though.
> --
> brian m. carlson (he/him or they/them)
> Toronto, Ontario, CA



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol
  2022-02-16 10:28               ` brian m. carlson
  2022-02-16 11:52                 ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
@ 2022-02-16 19:02                 ` Johannes Sixt
  1 sibling, 0 replies; 19+ messages in thread
From: Johannes Sixt @ 2022-02-16 19:02 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Torsten Bögershausen, Junio C Hamano, git

Am 16.02.22 um 11:28 schrieb brian m. carlson:
> The situation is that eol is in effect if and only if:
> 
> * text is set;
> * text is unspecified; or
> * text is auto, the file is detected as text, and the file has LF line
>   endings in the index.
> 
> Alternately, it has no effect if and only if:
> 
> * text is unset;
> * text is auto and the file is detected as binary; or
> * text is auto and the file is detected as text and has CRLF line
>   endings.
> 
> I'm not sure one reads significantly easier than the other.  I slightly
> prefer the former because it has fewer conditions with multiple nested
> entries, though.

I agree that the first version is easier to understand.

-- Hannes

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-02-16 19:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11  2:15 [PATCH 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
2022-01-11  2:15 ` [PATCH 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
2022-01-11  2:15 ` [PATCH 2/2] docs: correct documentation about eol attribute brian m. carlson
2022-01-11 18:30   ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
2022-01-11 22:40     ` brian m. carlson
2022-01-12 15:16       ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
2022-02-14  2:08 ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol brian m. carlson
2022-02-14  2:08   ` [PATCH v2 1/2] t0027: add tests for eol without text in .gitattributes brian m. carlson
2022-02-14  2:08   ` [PATCH v2 2/2] docs: correct documentation about eol attribute brian m. carlson
2022-02-14 14:52   ` [PATCH v2 0/2] Improvements to tests and docs for .gitattributes eol Derrick Stolee
2022-02-14 18:15   ` Junio C Hamano
2022-02-14 20:46     ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
2022-02-15  0:15       ` Junio C Hamano
2022-02-15  7:05         ` Johannes Sixt
2022-02-15 22:46           ` brian m. carlson
2022-02-16  7:00             ` Johannes Sixt
2022-02-16 10:28               ` brian m. carlson
2022-02-16 11:52                 ` Torsten =?unknown-8bit?Q?B=C3=B6gershausen?=
2022-02-16 19:02                 ` Johannes Sixt

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).