Re: [PATCH/RFC] blame: CRLF in the working tree and LF in the repo

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

 



On 04/27/2015 07:47 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> I suspect (I haven't looked very carefully for this round yet to be
>> sure, though) that it may turn out that the commit you are proposing
>> to revert was a misguided attempt to "fix" a non issue, or to break
>> the behaviour to match a mistaken expectation.  If that is the case
>> then definitely the reversion is a good idea, and you should argue
>> along that line of justification.
>>
>> We'd just be fixing an old misguided and bad change in such a case.
> The original says this:
>
>     blame: correctly handle files regardless of autocrlf
>     
>     If a file contained CRLF line endings in a repository with
>     core.autocrlf=input, then blame always marked lines as "Not
>     Committed Yet", even if they were unmodified.  Don't attempt to
>     convert the line endings when creating the fake commit so that blame
>     works correctly regardless of the autocrlf setting.
>     
>     Reported-by: Ephrim Khong <dr.khong@xxxxxxxxx>
>     Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
>     Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>
> But if autocrlf=input, then the end-user expectation is to keep the
> in-repository data with LF line endings.  If your tip-of-the-tree
> commit incorrectly has CRLF line endings, and if you were going to
> commit what is in the working tree on top, you would be correcting
> that mistake by turning the in-repository data into a text file with
> LF line endings, so "Not Committed Yet" _is_ the correct behaviour.
>
> So I think that the reverting that change is the right thing to do.
> It really was a change to break the behaviour to match a mistaken
> expectation, I would have to say.
Besides a better commit message (suggestions welcome),
What do you think about the following test cases for a V2 patch ?

test_expect_success 'create blamerepo' '
    test_create_repo blamerepo &&
    (
        cd blamerepo &&
        printf "testcase\r\n" >crlffile &&
        git -c core.autocrlf=false add crlffile &&
        git commit -m "add files" &&
        git -c core.autocrlf=false blame crlffile >crlfclean.txt
    )
'

test_expect_success 'blaming files with CRLF newlines in repo, core.autoclrf=input' '
    (
        cd blamerepo &&
        git -c core.autocrlf=input blame crlffile >actual &&
        grep "Not Committed Yet" actual
    )
'


test_expect_success 'blaming files with CRLF newlines core.autocrlf=true' '
    (
        cd blamerepo &&
        git -c core.autocrlf=true blame crlffile >actual &&
        test_cmp crlfclean.txt actual
    )
'

test_expect_success 'blaming files with CRLF newlines core.autocrlf=false' '
    (
        cd blamerepo &&
        git -c core.autocrlf=false blame crlffile >actual &&
        test_cmp crlfclean.txt actual
    )
'

--
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]