* [PATCH] docs: clarify how the text and text=auto attributes are different
@ 2023-04-21 5:56 Alex Henrie
2023-04-21 16:35 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Alex Henrie @ 2023-04-21 5:56 UTC (permalink / raw)
To: git, eyvind.bernhardsen, tboegi, gitster; +Cc: Alex Henrie
These two sentences are confusing because the description of the text
attribute sounds exactly the same as the description of the text=auto
attribute:
"Setting the text attribute on a path enables end-of-line normalization"
"When text is set to "auto", the path is marked for automatic
end-of-line conversion"
Unless the reader is already familiar with the two variants, there's a
high probability that they will think that "end-of-line normalization"
is the same thing as "automatic end-of-line conversion".
It's also confusing that the explanation of how end-of-line conversion
works is in the paragraph for text=auto even though it applies equally
to the text attribute which is described earlier.
On top of that, "When the file has been committed with CRLF, no
conversion is done" implies that normalization is only suppressed if the
file has been committed. In fact, running `git add` on a CRLF file,
adding the text attribute to the file, and running `git add` again does
not do anything to the line endings either.
Rephrase the documentation of text and text=auto to be clear about how
they are the same, how they are different, and in what cases
normalization is performed.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
Documentation/gitattributes.txt | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 39bfbca1ff..6db4ecd794 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -131,9 +131,12 @@ linkgit:git-config[1]).
Set::
- Setting the `text` attribute on a path enables end-of-line
- normalization and marks the path as a text file. End-of-line
- conversion takes place without guessing the content type.
+ Setting the `text` attribute on a path marks the path as a text
+ file, which enables end-of-line normalization: When a matching file
+ is added to the index, even if it has CRLF line endings in the
+ working directory, the file is stored in Git with LF line endings.
+ However, if the file was already in Git with CRLF endings, no
+ conversion is done.
Unset::
@@ -142,10 +145,9 @@ Unset::
Set to string value "auto"::
- When `text` is set to "auto", the path is marked for automatic
- end-of-line conversion. If Git decides that the content is
- text, its line endings are converted to LF on checkin.
- When the file has been committed with CRLF, no conversion is done.
+ When text is set to "auto", Git decides by itself whether the file
+ is text or binary. If it is text, line endings are converted as
+ described above. If it is binary, they are not.
Unspecified::
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] docs: clarify how the text and text=auto attributes are different
2023-04-21 5:56 [PATCH] docs: clarify how the text and text=auto attributes are different Alex Henrie
@ 2023-04-21 16:35 ` Junio C Hamano
2023-04-21 18:25 ` Torsten Bögershausen
2023-04-28 4:22 ` [PATCH v2] docs: rewrite the documentation of the text and eol attributes Alex Henrie
2 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2023-04-21 16:35 UTC (permalink / raw)
To: Alex Henrie; +Cc: git, eyvind.bernhardsen, tboegi
Alex Henrie <alexhenrie24@gmail.com> writes:
> These two sentences are confusing because the description of the text
> attribute sounds exactly the same as the description of the text=auto
> attribute:
>
> "Setting the text attribute on a path enables end-of-line normalization"
>
> "When text is set to "auto", the path is marked for automatic
> end-of-line conversion"
>
> Unless the reader is already familiar with the two variants, there's a
> high probability that they will think that "end-of-line normalization"
> is the same thing as "automatic end-of-line conversion".
True.
The prerequisite to understand these two seemingly contradicting
sentences is to know that "attribute" can be "set" (in the sense of
Boolean "yes") and can be "set to value" (here in this case, to the
string "auto"). They are treated differently.
It is very good that you decided to clarify it.
> It's also confusing that the explanation of how end-of-line conversion
> works is in the paragraph for text=auto even though it applies equally
> to the text attribute which is described earlier.
Good observation.
Instead of half-duplicating the same explanation in two places, it
is very good to do so just once, in the introductory description of
the 'text' attribute before the individual description of what
happens when the attribute is in any of the four states.
> On top of that, "When the file has been committed with CRLF, no
> conversion is done" implies that normalization is only suppressed if the
> file has been committed. In fact, running `git add` on a CRLF file,
> adding the text attribute to the file, and running `git add` again does
> not do anything to the line endings either.
True again. The conversion happens between the index and the
working tree, and "committing" does not have much say in the
process. Interestingly, the description of what happens when the
attribute is "Unset" (in the sense of setting it to Boolean "no")
uses a better terminology---"checkin or checkout", which is about
getting things out of or into the index.
It is very good that you decided to clarify it.
> Rephrase the documentation of text and text=auto to be clear about how
> they are the same, how they are different, and in what cases
> normalization is performed.
Thanks.
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
> Documentation/gitattributes.txt | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 39bfbca1ff..6db4ecd794 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -131,9 +131,12 @@ linkgit:git-config[1]).
>
> Set::
>
> - Setting the `text` attribute on a path enables end-of-line
> - normalization and marks the path as a text file. End-of-line
> - conversion takes place without guessing the content type.
> + Setting the `text` attribute on a path marks the path as a text
> + file, which enables end-of-line normalization: When a matching file
> + is added to the index, even if it has CRLF line endings in the
> + working directory, the file is stored in Git with LF line endings.
> + However, if the file was already in Git with CRLF endings, no
> + conversion is done.
I think most of the new text should go to the paragraph before this
hunk (i.e. "... its line endings are converted to ..."), where not
just CRLF->LF conversion is described, but how it does and does not
happen depending on the eol, core.eol, and core.autocrlf are set.
> Unset::
>
> @@ -142,10 +145,9 @@ Unset::
>
> Set to string value "auto"::
>
> - When `text` is set to "auto", the path is marked for automatic
> - end-of-line conversion. If Git decides that the content is
> - text, its line endings are converted to LF on checkin.
> - When the file has been committed with CRLF, no conversion is done.
> + When text is set to "auto", Git decides by itself whether the file
> + is text or binary. If it is text, line endings are converted as
> + described above. If it is binary, they are not.
This side is good. "As described above" can still be used as-is
even after the explanation of eol conversion is made before we start
enumerating Set/Unset/set to auto/unspecified.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] docs: clarify how the text and text=auto attributes are different
2023-04-21 5:56 [PATCH] docs: clarify how the text and text=auto attributes are different Alex Henrie
2023-04-21 16:35 ` Junio C Hamano
@ 2023-04-21 18:25 ` Torsten Bögershausen
2023-04-26 13:18 ` Alex Henrie
2023-04-28 4:22 ` [PATCH v2] docs: rewrite the documentation of the text and eol attributes Alex Henrie
2 siblings, 1 reply; 11+ messages in thread
From: Torsten Bögershausen @ 2023-04-21 18:25 UTC (permalink / raw)
To: Alex Henrie; +Cc: git, eyvind.bernhardsen, gitster
On Thu, Apr 20, 2023 at 11:56:41PM -0600, Alex Henrie wrote:
Thanks for that, fully agree with comments from Junio.
> Set::
>
> - Setting the `text` attribute on a path enables end-of-line
> - normalization and marks the path as a text file. End-of-line
> - conversion takes place without guessing the content type.
> + Setting the `text` attribute on a path marks the path as a text
> + file, which enables end-of-line normalization: When a matching file
> + is added to the index, even if it has CRLF line endings in the
> + working directory, the file is stored in Git with LF line endings.
Hm, I stumbled a little bit over the word "even" here.
But it makes things very clear, so this is good.
> + However, if the file was already in Git with CRLF endings, no
> + conversion is done.
But I think that this is wrong, unless I missed something.
Using
*.txt text
in .gitattributes
will convert CRLF into LF in the repo at the next git commit/git add
When you create a file with CRLF (no atttibutes), commit it into the repo,
and later add the attribute "text", the file may be reported as modified.
Not always. A `touch file.txt` typically asks Git to re-investigate things,
and it may show up as modified.
After a `git clone` it may be shown as modified or not.
End of side-note.
Setting
>
> Unset::
>
> @@ -142,10 +145,9 @@ Unset::
>
> Set to string value "auto"::
>
> - When `text` is set to "auto", the path is marked for automatic
> - end-of-line conversion. If Git decides that the content is
> - text, its line endings are converted to LF on checkin.
> - When the file has been committed with CRLF, no conversion is done.
> + When text is set to "auto", Git decides by itself whether the file
> + is text or binary. If it is text, line endings are converted as
> + described above. If it is binary, they are not.
Here we need to have the lines from above:
When text is set to "auto", Git decides by itself whether the file
is text or binary. If it is text, line endings are converted as
described above. If it is binary, they are not.
However, if the file was already in Git with CRLF endings, no
conversion is done.
I hope that I didn't messed up while reading this patch ?
Thanks again for bringing this up
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] docs: clarify how the text and text=auto attributes are different
2023-04-21 18:25 ` Torsten Bögershausen
@ 2023-04-26 13:18 ` Alex Henrie
0 siblings, 0 replies; 11+ messages in thread
From: Alex Henrie @ 2023-04-26 13:18 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, eyvind.bernhardsen, gitster
On Fri, Apr 21, 2023 at 12:26 PM Torsten Bögershausen <tboegi@web.de> wrote:
> Using
> *.txt text
> in .gitattributes
> will convert CRLF into LF in the repo at the next git commit/git add
>
> When you create a file with CRLF (no atttibutes), commit it into the repo,
> and later add the attribute "text", the file may be reported as modified.
> Not always. A `touch file.txt` typically asks Git to re-investigate things,
> and it may show up as modified.
> After a `git clone` it may be shown as modified or not.
> End of side-note.
Thank you very much for pointing that out. I had assumed that "text"
and "text=auto" behaved the same for text files, but they do not:
"text" normalizes line endings even if the file is already in Git.
That means that my proposed text is not correct, and I will have to
change it similarly to how you suggested below.
> > Set to string value "auto"::
> >
> > - When `text` is set to "auto", the path is marked for automatic
> > - end-of-line conversion. If Git decides that the content is
> > - text, its line endings are converted to LF on checkin.
> > - When the file has been committed with CRLF, no conversion is done.
> > + When text is set to "auto", Git decides by itself whether the file
> > + is text or binary. If it is text, line endings are converted as
> > + described above. If it is binary, they are not.
>
> Here we need to have the lines from above:
>
> When text is set to "auto", Git decides by itself whether the file
> is text or binary. If it is text, line endings are converted as
> described above. If it is binary, they are not.
> However, if the file was already in Git with CRLF endings, no
> conversion is done.
Yes, the note about what happens if the file is already in Git with
CRLF endings applies specifically to text=auto, so it needs to be in
the text=auto section. The documentation of the eol attribute actually
explains it more clearly:
"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."
I think that explanation needs to be moved from the eol attribute
documentation to the text attribute documentation and edited for
clarity. I will send a v2 that revises the documentation of both
attributes, taking into account your and Junio's feedback.
Thanks again,
-Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-21 5:56 [PATCH] docs: clarify how the text and text=auto attributes are different Alex Henrie
2023-04-21 16:35 ` Junio C Hamano
2023-04-21 18:25 ` Torsten Bögershausen
@ 2023-04-28 4:22 ` Alex Henrie
2023-04-28 18:05 ` Junio C Hamano
2023-04-30 13:19 ` Torsten Bögershausen
2 siblings, 2 replies; 11+ messages in thread
From: Alex Henrie @ 2023-04-28 4:22 UTC (permalink / raw)
To: git, eyvind.bernhardsen, tboegi, gitster; +Cc: Alex Henrie
These two sentences are confusing because the description of the text
attribute sounds exactly the same as the description of the text=auto
attribute:
"Setting the text attribute on a path enables end-of-line normalization"
"When text is set to "auto", the path is marked for automatic
end-of-line conversion"
Unless the reader is already familiar with the two variants, there's a
high probability that they will think that "end-of-line normalization"
is the same thing as "automatic end-of-line conversion".
It's also not clear that the phrase "When the file has been committed
with CRLF, no conversion is done" in the paragraph for text=auto does
not apply equally to the bare text attribute which is described earlier.
Moreover, it falsely implies that normalization is only suppressed if
the file has been committed. In fact, running `git add` on a CRLF file,
adding the text=auto attribute to the file, and running `git add` again
does not do anything to the line endings either.
On top of that, in several places the documentation for the eol
attribute sounds like it can force normalization on checkin and checkout
all by itself, but eol doesn't control normalization on checkin and
doesn't control normalization on checkout either unless accompanied by
the text attribute.
Rephrase the documentation of text, text=auto, eol, eol=crlf, and eol=lf
to be clear about how they are the same, how they are different, and in
what cases normalization is performed.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
v2: rewrite completely and rewrite the eol documentation too
---
Documentation/gitattributes.txt | 58 +++++++++++++++++++--------------
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 39bfbca1ff..bcea84f439 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -120,20 +120,22 @@ repository upon 'git add' and 'git commit'.
`text`
^^^^^^
-This attribute enables and controls end-of-line normalization. When a
-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 setting `core.autocrlf` to `true` or `input` overrides
-`core.eol` (see the definitions of those options in
-linkgit:git-config[1]).
+This attribute marks the path as a text file, which enables end-of-line
+normalization on checkin and possibly also checkout: When a matching
+file is added to the index, even if it has CRLF line endings in the
+working directory, the file is stored in the index with LF line endings.
+Conversely, when the file is copied from the index to the working
+directory, its line endings may be converted from LF to CRLF depending
+on the `eol` attribute, the Git config, and the platform (see
+explanation of `eol` below).
Set::
Setting the `text` attribute on a path enables end-of-line
- normalization and marks the path as a text file. End-of-line
- conversion takes place without guessing the content type.
+ normalization on checkin and checkout as described above. Line
+ endings are normalized in the index the next time the file is
+ checked in, even if the file was previously added to Git with CRLF
+ line endings.
Unset::
@@ -142,10 +144,11 @@ Unset::
Set to string value "auto"::
- When `text` is set to "auto", the path is marked for automatic
- end-of-line conversion. If Git decides that the content is
- text, its line endings are converted to LF on checkin.
- When the file has been committed with CRLF, no conversion is done.
+ When `text` is set to "auto", Git decides by itself whether the file
+ is text or binary. If it is text and the file was not already in
+ Git with CRLF endings, line endings are converted on checkin and
+ checkout as described above. Otherwise, no conversion is done on
+ checkin or checkout.
Unspecified::
@@ -162,23 +165,28 @@ 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`, 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.
+detected as text, and it is stored with LF endings in the index.
Set to string value "crlf"::
- This setting forces Git to normalize line endings for this
- file on checkin and convert them to CRLF when the file is
- checked out.
+ This setting converts the file's line endings in the working
+ directory to CRLF when the file is checked out.
Set to string value "lf"::
- This setting forces Git to normalize line endings to LF on
- checkin and prevents conversion to CRLF when the file is
- checked out.
+ This setting uses the same line endings in the working directory as
+ in the index, whether they are LF or CRLF. However, unless
+ `text=auto`, adding the file to the index again will normalize its
+ line endings to LF in the index.
+
+Unspecified::
+
+ If the `eol` attribute is unspecified for a file, its line endings
+ in the working directory are determined by the `core.autocrlf` or
+ `core.eol` configuration variable (see the definitions of those
+ options in linkgit:git-config[1]). The default if `text` is set but
+ neither of those variables is is `eol=lf` on Unix and `eol=crlf` on
+ Windows.
Backwards compatibility with `crlf` attribute
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-28 4:22 ` [PATCH v2] docs: rewrite the documentation of the text and eol attributes Alex Henrie
@ 2023-04-28 18:05 ` Junio C Hamano
2023-04-29 19:19 ` Alex Henrie
2023-04-30 13:19 ` Torsten Bögershausen
1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2023-04-28 18:05 UTC (permalink / raw)
To: Alex Henrie; +Cc: git, eyvind.bernhardsen, tboegi
Alex Henrie <alexhenrie24@gmail.com> writes:
> These two sentences are confusing because the description of the text
> attribute sounds exactly the same as the description of the text=auto
> attribute:
>
> "Setting the text attribute on a path enables end-of-line normalization"
>
> "When text is set to "auto", the path is marked for automatic
> end-of-line conversion"
>
> Unless the reader is already familiar with the two variants, there's a
> high probability that they will think that "end-of-line normalization"
> is the same thing as "automatic end-of-line conversion".
>
> It's also not clear that the phrase "When the file has been committed
> with CRLF, no conversion is done" in the paragraph for text=auto does
> not apply equally to the bare text attribute which is described earlier.
> Moreover, it falsely implies that normalization is only suppressed if
> the file has been committed. In fact, running `git add` on a CRLF file,
> adding the text=auto attribute to the file, and running `git add` again
> does not do anything to the line endings either.
>
> On top of that, in several places the documentation for the eol
> attribute sounds like it can force normalization on checkin and checkout
> all by itself, but eol doesn't control normalization on checkin and
> doesn't control normalization on checkout either unless accompanied by
> the text attribute.
>
> Rephrase the documentation of text, text=auto, eol, eol=crlf, and eol=lf
> to be clear about how they are the same, how they are different, and in
> what cases normalization is performed.
All good observations, appropriate motivation of the change.
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
> v2: rewrite completely and rewrite the eol documentation too
> ---
Thanks. I wonder helped-by tboegi@ might be in order, but hopefully
it may come during the review of this round.
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 39bfbca1ff..bcea84f439 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -120,20 +120,22 @@ repository upon 'git add' and 'git commit'.
> `text`
> ^^^^^^
>
> -This attribute enables and controls end-of-line normalization. When a
> -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 setting `core.autocrlf` to `true` or `input` overrides
> -`core.eol` (see the definitions of those options in
> -linkgit:git-config[1]).
> +This attribute marks the path as a text file, which enables end-of-line
> +normalization on checkin and possibly also checkout: When a matching
> +file is added to the index, even if it has CRLF line endings in the
> +working directory, the file is stored in the index with LF line endings.
> +Conversely, when the file is copied from the index to the working
> +directory, its line endings may be converted from LF to CRLF depending
> +on the `eol` attribute, the Git config, and the platform (see
> +explanation of `eol` below).
Four points.
1. Overall, the new text is clearly written, and I think it made the
updated description of "Set" and "Set to string 'auto'" easier to
understand.
2. "on checkin and possibly also checkout" may be redundant, as the
section in which the `text` attribute is described is all about
various conversions upon checking in (i.e. moving contents to the
index) and checking out (i.e. moving contents out of the index).
The introductory text for the whole section that does not define
what "check-in" and "check-out" are and instead loosely says
"commands such as" may want to be clarified, but that can be done
as a follow-on clean-up work after this patch lands.
3. It is unclear why we would want to mention "even if it has CRLF
line endings in the working directory". "Regardless of the line
endings in the working tree files" may slightly be better [*1*],
but I think the text reads better without it as storing contents
with possibly different end-of-line convention is the point of
"end-of-line normalization" in the first place.
"the file is stored in the index with LF line endings" may better
be rephrased to "the contents is stored, after converting to use
LF line endings if necessary, in the index", so that we can use
the verb "convert" to stress the point of setting this attribute.
4. This problem is shared with the original text, but "This
attribute marks the path as a text file" might be a bit
misleading. If it is left unspecified (i.e. unmentioned, or any
earlier settings by explicitly overriding them with "!text") or
worse yet, explicitly unset (i.e. "-text"), it marks the path as
a non text.
This attribute is used to mark the path as a text file, which
needs certain end-of-line normalization upon check-in and
check-out, or a non-text file, which do not.
I dunno if the extra verbosity is too much, though.
> Set::
>
> Setting the `text` attribute on a path enables end-of-line
> - normalization and marks the path as a text file. End-of-line
> - conversion takes place without guessing the content type.
> + normalization on checkin and checkout as described above. Line
> + endings are normalized in the index the next time the file is
> + checked in, even if the file was previously added to Git with CRLF
> + line endings.
"the next time" -> "every time", but that is probably moot, because
I would question the need for the whole sentence.
The last sentence, starting with "Line endigns are normalized", may
be redundant and we probably are better off without it. It would
risk becoming maintenance burden because we have to make sure it
stays in sync with what you wrote in the introductory text. "as
described above" before that sentence is clear enough.
> @@ -142,10 +144,11 @@ Unset::
>
> Set to string value "auto"::
>
> - When `text` is set to "auto", the path is marked for automatic
> - end-of-line conversion. If Git decides that the content is
> - text, its line endings are converted to LF on checkin.
> - When the file has been committed with CRLF, no conversion is done.
> + When `text` is set to "auto", Git decides by itself whether the file
> + is text or binary. If it is text and the file was not already in
> + Git with CRLF endings, line endings are converted on checkin and
> + checkout as described above. Otherwise, no conversion is done on
> + checkin or checkout.
Nice.
> @@ -162,23 +165,28 @@ 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`, 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.
> +detected as text, and it is stored with LF endings in the index.
This level of clarification is very much appreciated.
We may want to use the phrase "checking out" somewhere in
conjunction with "in the working directory" to be consistent with
how the section is titled. Something like
This attribute marks a path to use a specific line-ending style
in the working tree when it is checked out.
> Set to string value "crlf"::
>
> + This setting converts the file's line endings in the working
> + directory to CRLF when the file is checked out.
Nice.
> Set to string value "lf"::
>
> - This setting forces Git to normalize line endings to LF on
> - checkin and prevents conversion to CRLF when the file is
> - checked out.
> + This setting uses the same line endings in the working directory as
> + in the index, whether they are LF or CRLF. However, unless
> + `text=auto`, adding the file to the index again will normalize its
> + line endings to LF in the index.
First of all, "uses the same, except if ... then LF is used" is much
harder to understand than "use LF, except if ..." as an explanation
for "lf".
The description may be correct but is 'eol=lf' what controls how the
normalization is done upon checking in?
As you said in the explanation for the `eol` attribute as a whole,
`eol` is effective only when `text` says the path is a text file,
and that (i.e. the fact that `text` controls the end-of-line) makes
the path to use LF line endings, isn't it? The last sentence
starting with "However, unless" that talks about conversion going
into the index feels out of place.
There are some exception handling in the code for odd cases like the
contents with mixed line endings, a path set to use LF but the file
actually has CRLF, etc. While they are worth describing, I wonder
if they should be done in a separate paragraph.
> +Unspecified::
> +
> + If the `eol` attribute is unspecified for a file, its line endings
> + in the working directory are determined by the `core.autocrlf` or
> + `core.eol` configuration variable (see the definitions of those
> + options in linkgit:git-config[1]). The default if `text` is set but
> + neither of those variables is is `eol=lf` on Unix and `eol=crlf` on
> + Windows.
Nice.
[Footnote]
*1* The intent is not about special-casing CRLF but any other; if
Git were to be extended to support ancient MacOS, we would
convert even if it uses CR line endings.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-28 18:05 ` Junio C Hamano
@ 2023-04-29 19:19 ` Alex Henrie
2023-04-30 5:16 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Alex Henrie @ 2023-04-29 19:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, eyvind.bernhardsen, tboegi
On Fri, Apr 28, 2023 at 12:05 PM Junio C Hamano <gitster@pobox.com> wrote:
> Thanks. I wonder helped-by tboegi@ might be in order, but hopefully
> it may come during the review of this round.
Sure, I'll add a Helped-by line in the next revision.
> 1. Overall, the new text is clearly written, and I think it made the
> updated description of "Set" and "Set to string 'auto'" easier to
> understand.
I'm glad you like it!
> 2. "on checkin and possibly also checkout" may be redundant, as the
> section in which the `text` attribute is described is all about
> various conversions upon checking in (i.e. moving contents to the
> index) and checking out (i.e. moving contents out of the index).
For the `text` attribute's behavior to make sense, the first thing a
newbie has to understand is that `text` definitely enables
normalization on checkin and might or might not enable normalization
on checkout. The original text made it sound like `text` only controls
checkin and `eol` controls checkout, a false impression that is
especially misleading because on Linux and OS X, `text` without `eol`
(or core.autocrlf or core.eol) does only enable normalization on
checkin. The new text may be slightly redundant, but it makes the
important part crystal clear.
> 3. It is unclear why we would want to mention "even if it has CRLF
> line endings in the working directory". "Regardless of the line
> endings in the working tree files" may slightly be better [*1*],
> but I think the text reads better without it as storing contents
> with possibly different end-of-line convention is the point of
> "end-of-line normalization" in the first place.
> *1* The intent is not about special-casing CRLF but any other; if
> Git were to be extended to support ancient MacOS, we would
> convert even if it uses CR line endings.
I called out CRLF precisely because I was thinking about platforms
that use CR instead of LF or CRLF. I wanted to make it clear that if
you add a file that was written on or for an ancient Mac or Commodore,
the line endings are not normalized. Git can be used (and probably is
being used by someone) to version-control files for those platforms
without actually running on those platforms.
Then again, maybe the fact that the `text` attribute does not
normalize CR line endings is a bug in Git, and we shouldn't advertise
it in the documentation as if it were intended behavior. What do you
think?
> "the file is stored in the index with LF line endings" may better
> be rephrased to "the contents is stored, after converting to use
> LF line endings if necessary, in the index", so that we can use
> the verb "convert" to stress the point of setting this attribute.
The phrase "if necessary" would be a bit confusing. What makes
conversion on checkin "necessary"? The reader would wonder if it
depends on the Git config and the platform like conversion on checkout
does.
Would you be OK with your proposed wording minus "if necessary"?
> 4. This problem is shared with the original text, but "This
> attribute marks the path as a text file" might be a bit
> misleading. If it is left unspecified (i.e. unmentioned, or any
> earlier settings by explicitly overriding them with "!text") or
> worse yet, explicitly unset (i.e. "-text"), it marks the path as
> a non text.
>
> This attribute is used to mark the path as a text file, which
> needs certain end-of-line normalization upon check-in and
> check-out, or a non-text file, which do not.
>
> I dunno if the extra verbosity is too much, though.
Hmm, I would find your proposed wording misleading because it seems to
imply that the default is `text=auto` and that `-text` (or a config
variable) is necessary to turn end-of-line normalization off. In my
opinion it's already clear enough that the attribute is unset by
default and that `-text` unsets a previous `text`.
> > Set::
> >
> > Setting the `text` attribute on a path enables end-of-line
> > - normalization and marks the path as a text file. End-of-line
> > - conversion takes place without guessing the content type.
> > + normalization on checkin and checkout as described above. Line
> > + endings are normalized in the index the next time the file is
> > + checked in, even if the file was previously added to Git with CRLF
> > + line endings.
>
> "the next time" -> "every time", but that is probably moot, because
> I would question the need for the whole sentence.
>
> The last sentence, starting with "Line endigns are normalized", may
> be redundant and we probably are better off without it. It would
> risk becoming maintenance burden because we have to make sure it
> stays in sync with what you wrote in the introductory text. "as
> described above" before that sentence is clear enough.
I added that sentence to make the difference between `text` and
`text=auto` more clear. If all we have is the introductory paragraph,
it sounds like Git only converts CRLF to LF on checkin if it had
converted LF to CRLF on checkout. However, if you feel strongly that
the sentence is unhelpful here, I'll remove it.
> We may want to use the phrase "checking out" somewhere in
> conjunction with "in the working directory" to be consistent with
> how the section is titled. Something like
>
> This attribute marks a path to use a specific line-ending style
> in the working tree when it is checked out.
That's fine.
> > Set to string value "lf"::
> >
> > - This setting forces Git to normalize line endings to LF on
> > - checkin and prevents conversion to CRLF when the file is
> > - checked out.
> > + This setting uses the same line endings in the working directory as
> > + in the index, whether they are LF or CRLF. However, unless
> > + `text=auto`, adding the file to the index again will normalize its
> > + line endings to LF in the index.
>
> First of all, "uses the same, except if ... then LF is used" is much
> harder to understand than "use LF, except if ..." as an explanation
> for "lf".
>
> The description may be correct but is 'eol=lf' what controls how the
> normalization is done upon checking in?
>
> As you said in the explanation for the `eol` attribute as a whole,
> `eol` is effective only when `text` says the path is a text file,
> and that (i.e. the fact that `text` controls the end-of-line) makes
> the path to use LF line endings, isn't it? The last sentence
> starting with "However, unless" that talks about conversion going
> into the index feels out of place.
That's a good point, it would be more clear here to explicitly say
that the setting affects checkout, and not mention what happens on
checkin (which was described previously in the `text` documentation).
Also, if we delete the sentence here then the seemingly redundant
sentence that starts with "Line endings are normalized" wouldn't be as
redundant.
> There are some exception handling in the code for odd cases like the
> contents with mixed line endings, a path set to use LF but the file
> actually has CRLF, etc. While they are worth describing, I wonder
> if they should be done in a separate paragraph.
Probably best done in a separate patch after this rewrite is done.
Wow, that was a LOT of feedback on a relatively short piece of text.
Are you sure you don't want to rewrite the documentation yourself? ;-)
-Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-29 19:19 ` Alex Henrie
@ 2023-04-30 5:16 ` Junio C Hamano
2023-05-01 0:40 ` Alex Henrie
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2023-04-30 5:16 UTC (permalink / raw)
To: Alex Henrie; +Cc: git, eyvind.bernhardsen, tboegi
Alex Henrie <alexhenrie24@gmail.com> writes:
> The new text may be slightly redundant, but it makes the
> important part crystal clear.
I tend to disagree and think it is a bit too redundant, but we do
not have to agree on everything---after all you are doing all the
work, and it is your motivation and your topic.
> Then again, maybe the fact that the `text` attribute does not
> normalize CR line endings is a bug in Git, and we shouldn't advertise
> it in the documentation as if it were intended behavior. What do you
> think?
Just not mentioning CRLF specifically would be sufficient. When
(and if) somebody actually comes and complains, we can say "CR
delimited files are not considered text these days, we aren't doing
MacOS System 7" ;-).
> The phrase "if necessary" would be a bit confusing. What makes
> conversion on checkin "necessary"? The reader would wonder if it
> depends on the Git config and the platform like conversion on checkout
> does.
>
> Would you be OK with your proposed wording minus "if necessary"?
I added it only because it would not be necessary if the original
already uses LF line endings, in which you do not have to do any
converison. As long as it is clear that no conversion happens in
such a case without saying, I am perfectly fine to drop it.
>> There are some exception handling in the code for odd cases like the
>> contents with mixed line endings, a path set to use LF but the file
>> actually has CRLF, etc. While they are worth describing, I wonder
>> if they should be done in a separate paragraph.
>
> Probably best done in a separate patch after this rewrite is done.
What I meant was that that your "unless text=auto" and everything
after it was one of thoese exception handling that should be dealt
in a separate paragraph. Leaving it as a follow-on work is fine.
> Wow, that was a LOT of feedback on a relatively short piece of text.
> Are you sure you don't want to rewrite the documentation yourself? ;-)
Absolutely not. It is not my itch to update this document.
My itch is to make sure any change makes the resulting text better
than the previous one ;-).
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-28 4:22 ` [PATCH v2] docs: rewrite the documentation of the text and eol attributes Alex Henrie
2023-04-28 18:05 ` Junio C Hamano
@ 2023-04-30 13:19 ` Torsten Bögershausen
2023-04-30 23:43 ` Alex Henrie
1 sibling, 1 reply; 11+ messages in thread
From: Torsten Bögershausen @ 2023-04-30 13:19 UTC (permalink / raw)
To: Alex Henrie; +Cc: git, eyvind.bernhardsen, gitster
On Thu, Apr 27, 2023 at 10:22:21PM -0600, Alex Henrie wrote:
Thanks for picking this up.
There had been some comments from Junio, I haven't had time to look at
them yet.
Some of my comments inline, lets see how we can converge things.
> These two sentences are confusing because the description of the text
> attribute sounds exactly the same as the description of the text=auto
> attribute:
>
> "Setting the text attribute on a path enables end-of-line normalization"
>
> "When text is set to "auto", the path is marked for automatic
> end-of-line conversion"
>
> Unless the reader is already familiar with the two variants, there's a
> high probability that they will think that "end-of-line normalization"
> is the same thing as "automatic end-of-line conversion".
>
> It's also not clear that the phrase "When the file has been committed
> with CRLF, no conversion is done" in the paragraph for text=auto does
> not apply equally to the bare text attribute which is described earlier.
> Moreover, it falsely implies that normalization is only suppressed if
> the file has been committed. In fact, running `git add` on a CRLF file,
> adding the text=auto attribute to the file, and running `git add` again
> does not do anything to the line endings either.
>
> On top of that, in several places the documentation for the eol
> attribute sounds like it can force normalization on checkin and checkout
> all by itself, but eol doesn't control normalization on checkin and
> doesn't control normalization on checkout either unless accompanied by
> the text attribute.
>
> Rephrase the documentation of text, text=auto, eol, eol=crlf, and eol=lf
> to be clear about how they are the same, how they are different, and in
> what cases normalization is performed.
>
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
> v2: rewrite completely and rewrite the eol documentation too
> ---
> Documentation/gitattributes.txt | 58 +++++++++++++++++++--------------
> 1 file changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 39bfbca1ff..bcea84f439 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -120,20 +120,22 @@ repository upon 'git add' and 'git commit'.
> `text`
> ^^^^^^
>
> -This attribute enables and controls end-of-line normalization. When a
Hm, not only, I think. The terminologie is probably not very well specified.
I would say that it "controls end-of-line conversion".
There are 2 type of conversions, from CRLF into LF and LF into CRLF.
The CRLF -> LF conversion happens only at `git commit`
(strictly speaking already at `git add`) and is called normalization.
Because in Git a "normalized" file has LF in the repo (and index).
The term normalize has even been add to 2 commands:
`git add --renormalize .`
inspired by
`git merge -Xrenormalize`
> -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.
In general, the `eol` attribute can be used by more that a single file.
And that is what we (or at least myself) recommend to do, in a kind
of best practice fashion, as an example:
*.sh text eol=lf
The core.eol is the fallback, when the eol attribute is not specified.
But the we look at core.autocrlf, before looking at core.eol,
as pointed out below.
And if none of them is set, Git uses the platform native setting,
whih is CRLF for Windows and LF for all other systems.
> -Note that setting `core.autocrlf` to `true` or `input` overrides
> -`core.eol` (see the definitions of those options in
Looking with fresh eyes: I am not sure if like this historical construct.
First we say the the "core.eol" sets the line endings (if not defined in
the attribute) and the we say that core.autcrlf overrides core.eol
This is mainly due to historically resons.
I think that things goes like this:
When text or text=auto (and Git identifies the file as text),
and the eol attribute is not set, then:
core.autocrlf=true gives CRLF
core.autocrlf=input give LF
core.autocrlf=false looks at core.eol:
core.eol=clrf gives CRLF
core.eol=lf give LF
core.eol unset gives the platform default
> -linkgit:git-config[1]).
> +This attribute marks the path as a text file, which enables end-of-line
> +normalization on checkin and possibly also checkout: When a matching
As said before.
...normalization on checkin and possibly conversion at checkout...
or
... conversion on checkin and possibly also checkout...
> +file is added to the index, even if it has CRLF line endings in the
> +working directory, the file is stored in the index with LF line endings.
> +Conversely, when the file is copied from the index to the working
"copied" is not an ideal word here:
We may specify a filter and/or an encoding as well.
Would "transferred and possibly filtered/encoded" be better ?
> +directory, its line endings may be converted from LF to CRLF depending
> +on the `eol` attribute, the Git config, and the platform (see
> +explanation of `eol` below).
>
> Set::
>
> Setting the `text` attribute on a path enables end-of-line
> - normalization and marks the path as a text file. End-of-line
> - conversion takes place without guessing the content type.
> + normalization on checkin and checkout as described above. Line
> + endings are normalized in the index the next time the file is
> + checked in, even if the file was previously added to Git with CRLF
> + line endings.
>
> Unset::
>
> @@ -142,10 +144,11 @@ Unset::
>
> Set to string value "auto"::
>
> - When `text` is set to "auto", the path is marked for automatic
> - end-of-line conversion. If Git decides that the content is
> - text, its line endings are converted to LF on checkin.
> - When the file has been committed with CRLF, no conversion is done.
> + When `text` is set to "auto", Git decides by itself whether the file
> + is text or binary. If it is text and the file was not already in
> + Git with CRLF endings, line endings are converted on checkin and
> + checkout as described above. Otherwise, no conversion is done on
> + checkin or checkout.
Side note: We previously talked about files. path is better.
>
> Unspecified::
>
> @@ -162,23 +165,28 @@ 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`, 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.
... Or `git add --renormalize <path> is run.
Adding the path to the index again will normalize
> -the line endings in the index.
> +detected as text, and it is stored with LF endings in the index.
>
> Set to string value "crlf"::
>
> - This setting forces Git to normalize line endings for this
> - file on checkin and convert them to CRLF when the file is
> - checked out.
> + This setting converts the file's line endings in the working
> + directory to CRLF when the file is checked out.
>
> Set to string value "lf"::
>
> - This setting forces Git to normalize line endings to LF on
> - checkin and prevents conversion to CRLF when the file is
> - checked out.
> + This setting uses the same line endings in the working directory as
> + in the index, whether they are LF or CRLF. However, unless
> + `text=auto`, adding the file to the index again will normalize its
> + line endings to LF in the index.
> +
> +Unspecified::
> +
> + If the `eol` attribute is unspecified for a file, its line endings
> + in the working directory are determined by the `core.autocrlf` or
> + `core.eol` configuration variable (see the definitions of those
> + options in linkgit:git-config[1]). The default if `text` is set but
> + neither of those variables is is `eol=lf` on Unix and `eol=crlf` on
> + Windows.
>
> Backwards compatibility with `crlf` attribute
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> --
> 2.40.0
>
Thanks again for working on this.
/Torsten
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-30 13:19 ` Torsten Bögershausen
@ 2023-04-30 23:43 ` Alex Henrie
0 siblings, 0 replies; 11+ messages in thread
From: Alex Henrie @ 2023-04-30 23:43 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, eyvind.bernhardsen, gitster
On Sun, Apr 30, 2023 at 7:19 AM Torsten Bögershausen <tboegi@web.de> wrote:
>
> On Thu, Apr 27, 2023 at 10:22:21PM -0600, Alex Henrie wrote:
> > -This attribute enables and controls end-of-line normalization. When a
>
> Hm, not only, I think. The terminologie is probably not very well specified.
> I would say that it "controls end-of-line conversion".
> There are 2 type of conversions, from CRLF into LF and LF into CRLF.
> The CRLF -> LF conversion happens only at `git commit`
> (strictly speaking already at `git add`) and is called normalization.
> Because in Git a "normalized" file has LF in the repo (and index).
> The term normalize has even been add to 2 commands:
>
> `git add --renormalize .`
> inspired by
> `git merge -Xrenormalize`
Yes, we should probably be consistent about using the word "normalize"
to refer to conversion to LF and the word "convert" to refer to
conversion to CRLF or conversion in general. That would also allow us
to use the word "normalize" for emphasis without wondering whether to
add "if necessary", because normalization certainly does not imply
conversion if the file is already normalized.
> > -Note that setting `core.autocrlf` to `true` or `input` overrides
> > -`core.eol` (see the definitions of those options in
> > -linkgit:git-config[1]).
>
> Looking with fresh eyes: I am not sure if like this historical construct.
> First we say the the "core.eol" sets the line endings (if not defined in
> the attribute) and the we say that core.autcrlf overrides core.eol
>
> This is mainly due to historically resons.
> I think that things goes like this:
> When text or text=auto (and Git identifies the file as text),
> and the eol attribute is not set, then:
> core.autocrlf=true gives CRLF
> core.autocrlf=input give LF
> core.autocrlf=false looks at core.eol:
> core.eol=clrf gives CRLF
> core.eol=lf give LF
> core.eol unset gives the platform default
Yes, core.autocrlf takes precedence over core.eol. That's explained in
the git-config documentation and doesn't need to be explained again in
the attribute documentation. However, in the documentation for
unspecified `eol`, I did make sure to name `eol`, core.autocrlf, and
core.eol in order of precedence.
> > +file is added to the index, even if it has CRLF line endings in the
> > +working directory, the file is stored in the index with LF line endings.
> > +Conversely, when the file is copied from the index to the working
> "copied" is not an ideal word here:
> We may specify a filter and/or an encoding as well.
> Would "transferred and possibly filtered/encoded" be better ?
Although "transferred and possibly filtered/encoded" might be
technically more accurate, it is a mouthful. For readability, it would
be better to keep the documentation concise. It's clear in this
sentence that "copy" does not mean a byte-for-byte copy (because the
line endings may be changed), which leaves open the possibility that
the file may be transformed in other ways during checkout.
> > + When `text` is set to "auto", Git decides by itself whether the file
> > + is text or binary. If it is text and the file was not already in
> > + Git with CRLF endings, line endings are converted on checkin and
> > + checkout as described above. Otherwise, no conversion is done on
> > + checkin or checkout.
>
> Side note: We previously talked about files. path is better.
It depends: When talking about applying the attribute to a path, path
is better, but the actual transformation happens on the file contents.
The file is what's text or binary and the file is what's transformed;
the path is always text and never transformed.
Thanks for the feedback. I will send a v3 that takes into account your
feedback and Junio's.
-Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] docs: rewrite the documentation of the text and eol attributes
2023-04-30 5:16 ` Junio C Hamano
@ 2023-05-01 0:40 ` Alex Henrie
0 siblings, 0 replies; 11+ messages in thread
From: Alex Henrie @ 2023-05-01 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, eyvind.bernhardsen, tboegi
On Sat, Apr 29, 2023 at 11:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > The new text may be slightly redundant, but it makes the
> > important part crystal clear.
>
> I tend to disagree and think it is a bit too redundant, but we do
> not have to agree on everything---after all you are doing all the
> work, and it is your motivation and your topic.
I just now realized that `eol=crlf` works when `text` is unspecified.
In that light, the phrase "on checkin and possibly also checkout"
seems to go a little too far towards implying that the eol attribute
and core.autocrlf config variable depend on the text attribute being
set. Omitting that phrase would leave the introduction with a better
balance between precision and brevity, and the remainder of the
documentation would still be there to flesh out the details.
> > Then again, maybe the fact that the `text` attribute does not
> > normalize CR line endings is a bug in Git, and we shouldn't advertise
> > it in the documentation as if it were intended behavior. What do you
> > think?
>
> Just not mentioning CRLF specifically would be sufficient. When
> (and if) somebody actually comes and complains, we can say "CR
> delimited files are not considered text these days, we aren't doing
> MacOS System 7" ;-).
I don't think I would dare say "CR-delimited text is not text" to a
retro computing enthusiast. But I'm fine with not mentioning CRLF in
this sentence, and when the angry mob shows up on my doorstep, I will
just send them your way ;-)
-Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-01 0:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21 5:56 [PATCH] docs: clarify how the text and text=auto attributes are different Alex Henrie
2023-04-21 16:35 ` Junio C Hamano
2023-04-21 18:25 ` Torsten Bögershausen
2023-04-26 13:18 ` Alex Henrie
2023-04-28 4:22 ` [PATCH v2] docs: rewrite the documentation of the text and eol attributes Alex Henrie
2023-04-28 18:05 ` Junio C Hamano
2023-04-29 19:19 ` Alex Henrie
2023-04-30 5:16 ` Junio C Hamano
2023-05-01 0:40 ` Alex Henrie
2023-04-30 13:19 ` Torsten Bögershausen
2023-04-30 23:43 ` Alex Henrie
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).