Re: [PATCH] Fix crlf attribute handling to match documentation

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

 



Andy Parkins <andyparkins@xxxxxxxxx> writes:

> gitattributes.txt says, of the crlf attribute:
>
>  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.
>
> That is to say that the crlf attribute does not force the file to have
> CRLF line endings, instead it removes the autocrlf guesswork and forces
> the file to be treated as text.  Then, whatever line ending is defined
> by the autocrlf setting is applied.

Thanks; I looked at the patch (although I am still _physically_
at work ;-).  I think your code is correct but the explanation
is slightly misleading.

> However, that is not what convert.c was doing.  The conversion to CRLF
> was being skipped in crlf_to_worktree() when the following condition was
> true:
>
>  action == CRLF_GUESS && auto_crlf <= 0

The check you modified in the first hunk is not the above '<='
comparison but is this:

	(action == CRLF_GUESS && !auto_crlf)

and "core.autocrlf = input" makes "auto_crlf = -1", so when
action is not GUESS, or even when action is GUESS, if the config
is set to "input", the if() statement you patched in the first
hunk should not trigger.  The above description is different from
what the code was doing.

The logic (in crlf_to_git, which is the input codepath) should be:

	* if action is BINARY, do nothing (obviously -- and the
          code gets this right).

	* if action is GUESS, do nothing if config says false
          (we want 'input' and 'true' to apply the munging after
          guessing).

	* if action is TEXT or INPUT, do not guess but do CRLF
          only on platforms that need it -- which means where
          auto_crlf is -1 (input) or 1 (true).  Otherwise do not
          do the conversion.

The original code gets the third case incorrectly, and your
patch fixes it by returning early in that case as well.

The output codepath is the same.  Regardless of action
(GUESS/TEXT/INPUT), we will not do anything if config says 'false'
or 'input', so removing the check with "action == GUESS" is the
right thing.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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