Re: [PATCH v4 1/2] git-p4: add test case for "Translation of file content failed" error

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

 



On 22 Sep 2015, at 01:54, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:

> On Mon, Sep 21, 2015 at 7:03 PM, Lars Schneider
> <larsxschneider@xxxxxxxxx> wrote:
>> On 21 Sep 2015, at 20:09, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> larsxschneider@xxxxxxxxx writes:
>>>> +test_expect_success 'init depot with UTF-16 encoded file and artificially remove BOM' '
>>>> +    (
>>>> +            cd "db" &&
>>>> +            p4d -jc &&
>>>> +            # P4D automatically adds a BOM. Remove it here to make the file invalid.
>>>> +            sed -e "$ d" depot/file1,v >depot/file1,v.new &&
>>> 
>>> Do you need the space between the address $ (i.e. the last line) and
>>> operation 'd' (i.e. delete it)?  I am asking because that looks very
>>> unusual at least in our codebase.
>> 
>> Well, I am no “sed” pro. I have to admit that I found this snippet
>> on the Internet and it just worked. If I remove the space then it
>> does not work. I was not yet able to figure out why… anyone an idea?
> 
> Yes, it's because $d is a variable reference, even within double
> quotes. Typically, one uses single quotes around the sed argument to
> suppress this sort of undesired behavior. Since the entire test body
> is already within single quotes, however, changing the sed argument to
> use single quotes, rather than double, will require escaping them:
> 
>    sed -e \'$d\' depot/file...
> 
> Aside: You could also drop the unnecessary quotes from the 'cd' argument.

Thanks for the explanation. Plus you are correct with the quotes around “db”… just a habit.

@Junio: 
If it is no extra work for you, you can remove the quotes around “db”. I can also create a new patch roll including the sed change and the quote change if it is easier for you.

Best,
Lars

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