Re: [PATCH] Respect crlf attribute in "git add" even if core.autocrlf has not been set

From: Steffen Prohaska <prohaska@zib.de>
Date: Jul 24, 2008, 06:06 UTC
Message-ID: <FCAEAB20-750E-47B9-B58D-9BB0CB1EEAFF@zib.de>
In-Reply-To: <20080723114022.GP2925@dpotapov.dyndns.org>

On Jul 23, 2008, at 1:40 PM, Dmitry Potapov wrote:

Show 28 quoted lines

> On Wed, Jul 23, 2008 at 07:49:20AM +0200, Steffen Prohaska wrote:
>>
>> On Jul 23, 2008, at 3:31 AM, Johannes Schindelin wrote:
>>
>>>
>>> -	if ((action == CRLF_BINARY) || !auto_crlf || !len)
>>> +	if ((action == CRLF_BINARY) || (!auto_crlf && action < 0) || !len)
>>
>> I think we should strictly follow the documentation, so this should  
>> read
>>
>> +       if ((action == CRLF_BINARY) || (!auto_crlf && action !=
>> CRLF_INPUT) || !len)
>
> Well, your expression is correct if we choose to strictly follow to  
> what
> the current documentation says, but it is not well written in this  
> place,
> and, more importantly, I don't see any use case where you would want  
> to
> set crlf=input in .gitattributes, because it is shared among users on
> different platforms. What you want to specify in it is whether a  
> file is
> text or binary. If crlf is set, it means a text file; if unset, it  
> is a
> binary file. Regardless of what autocrlf value, I don't see why a text
> file should be checked in with CRs. So, Dscho's patch makes more sense
> to me.  This requires correction to the documentation though:

Ok, this and Dscho's earlier mail convinced me. My patch does not make sense at all. I should have had thought a bit harder ;-)

I have however a related question:

Dscho, Is the following your use case?

  "I am the maintainer of this project.  I know that this project needs
   crlf conversion, because it is a cross-platform project.  Therefore,
   I want to force crlf conversion for this specific project, even if
   the user did not configure core.autocrlf=input on Unix."

Your patch provides a solution, though not a very comfortable one. With your patch applied, you could explicitly list all files (or filetypes) that are text and mark them with 'crlf'. Git would not let CRLFs enter the repository for these files. You could also specify 'crlf=input', but I don't understand why you want to specify this. Anyway, you would need to explicitly list all text filetypes and explicitly *not list* the binary filetypes because they must not be converted. This is no very comfortable, no?

I think what you really want to do is to tell git that the *automatic* crlf detection shall be used for all files of your specific project, even if the user did not configure core.autocrlf=input. This would avoid listing each filetype separately.

Maybe we could allow 'crlf=guess' in .gitattributes with the following documentation:

-- 8< -- Set to string value "guess"::

	Setting `guess` tells git to apply conversion upon checkin
         if the file content looks like text.  On checkout, however,
         git applies the conversion specified in `core.autocrlf`.
         Setting `guess` can be useful to force automatic file type
         detection for a specific project.
-- >8 --

Show 17 quoted lines

> diff --git a/Documentation/gitattributes.txt b/Documentation/ 
> gitattributes.txt
> index d7b4114..448857b 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -99,9 +99,9 @@ This attribute controls the line-ending convention.
> Set::
>
> 	Setting the `crlf` attribute on a path is meant to mark
> -	the path as a "text" file.  'core.autocrlf' conversion
> -	takes place without guessing the content type by
> -	inspection.
> +	the path as a "text" file. Line endings in a text file
> +	are converted to LF upon checkin, and if 'core.autocrlf'
> +	is true then to CRLF upon checkout.
>
> Unset::

Makes sense.

	Steffen