Re: [PATCH v3 1/1] docs: rewrite the documentation of the text and eol attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 1, 2023 at 10:17 PM Torsten Bögershausen <tboegi@xxxxxx> wrote:
>
> This looks much better, thanks.
> I have some minor comments:
> a) The commit message from above:
> >  docs: rewrite the documentation of the text and eol attributes
>
> The word "doc" is used 2 times, that feel a little bit redundant
> (and should start with a uppercase letter)
>
> "Docs: rewrite the documentation of the text and eol attributes"
>
> Or may be shorter:
> "Rewrite the documentation of the text and eol attributes"

The commit message is modeled after 8c591dbfce, which is incidentally
the commit that introduced the confusing wording discussed below.
Unless there is a convention on how to write commit messages like
this, I would rather follow the existing precedent.

> On Sun, Apr 30, 2023 at 08:35:33PM -0600, Alex Henrie wrote:
> > These two sentences are confusing because the description of the text
> > attribute sounds exactly the same as the description of the text=auto
> > attribute:
>
> The word "These" is somewhat dangling: Which ones ?
> May be "The following two sentences" ?

The colon indicates that the sentence refers to the following sentences.

> > -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
> > +conversion: When a matching file is added to the index, the file's line
> > +endings are normalized to LF in the index.  Conversely, when the file is
> > +copied from the index to the working directory, its line endings may be
> I still stumble accross "copied". May be shorter:
>
> "the file is written into the working directory"

Maybe, but then it almost sounds like Git watches the directory for
changes and transforms files as soon as the user creates them.
Conversion only happens when writing files _from the index_. Moreover,
the word "copy" does not always mean a byte-for-byte copy, and clearly
does not mean that here.

> > -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.
> > +This attribute marks a path to use a specific line-ending style in the
> > +working tree when it is checked out.
> It enables even the normalization at checkin, see
> $ mkdir ttt
> $ cd ttt
> $ git init
> $ echo "*.sh eol=lf" >.gitattributes
> $ printf '#!/bin/sh\r\necho hello\r\n' >xx.sh
> $ git add xx.sh
> warning: CRLF will be replaced by LF in xx.sh.
> The file will have its original line endings in your working directory

This has got to be the most confusing Git feature ever. Clearly I did
not understand how it works.

> > + 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.
> It took me a while to understand it.
> Should the "," after "unspecified" be removed ?
>
> Or, should we write:
>
> The `eol` attribute automatically sets `text`, unless `-text`, `binary` or
> `text=auto` is specified.

Your proposed wording makes the relationship between `text` and `eol`
far more clear, and it avoids the need to repeat the detailed
explanation of how normalization on checkin works. However, I think we
still need to say in the introduction that `eol` does not work without
`text`. How about this:

It has effect only if `text` or `text=auto` is set (see above), but
specifying `eol` automatically sets `text` if `text` was left
unspecified.

I've CCed the author of the original sentence on this email in case he
wants to give any feedback on the rewrite.

> > +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.
>
> That's good - I wonder if everyone understands Linux, MacOs and others as Unix.
>
> May be something like this:
> The default, if `text` is set but neither of those variables, is `eol=crlf` on
> Windows and `eol=lf` on all other systems.

That's fine, we can say "all other platforms" instead of "Unix" here.
It would only become a problem if someone ports Git to DOS, which
seems unlikely.

Thanks for the insight and help,

-Alex




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux