threads / patch / 35220

patchChange sed i\ usage to something Solaris' sed can handle

Subject: [PATCH] Change sed i\ usage to something Solaris' sed can handle

## tl;dr

6 messages between Oct 27, 2013 and Nov 3, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Ben Walton· Oct 27, 2013, 21:26 UTC · lore

Solaris' sed was choking on the i\ commands used in t4015-diff-whitespace as it couldn't parse the program properly. Modify two uses of sed that worked in GNU sed but not Solaris' (/usr/bin or /usr/xpg4/bin) to an equivalent form that is handled properly by both.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
 t/t4015-diff-whitespace.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Show changes to t/t4015-diff-whitespace.sh +4 −2

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 3fb4b97..0126154 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -145,7 +145,8 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect
 test_expect_success 'ignore-blank-lines: only new lines' '
 	test_seq 5 >x &&
 	git update-index x &&
-	test_seq 5 | sed "/3/i \\
+	test_seq 5 | sed "/3/i\\
+\
 " >x &&
 	git diff --ignore-blank-lines >out &&
 	>expect &&
@@ -155,7 +156,8 @@ test_expect_success 'ignore-blank-lines: only new lines' '
 test_expect_success 'ignore-blank-lines: only new lines with space' '
 	test_seq 5 >x &&
 	git update-index x &&
-	test_seq 5 | sed "/3/i \ " >x &&
+	test_seq 5 | sed "/3/i\\
+ " >x &&
 	git diff -w --ignore-blank-lines >out &&
 	>expect &&
 	test_cmp out expect

-- 
1.8.1.2

Andreas Schwab· Oct 28, 2013, 17:39 UTC · re: Ben Walton · lore

Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle

Ben Walton <bdwalton@gmail.com> writes:

Show 12 quoted lines

> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index 3fb4b97..0126154 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -145,7 +145,8 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect
>  test_expect_success 'ignore-blank-lines: only new lines' '
>  	test_seq 5 >x &&
>  	git update-index x &&
> -	test_seq 5 | sed "/3/i \\
> +	test_seq 5 | sed "/3/i\\
> +\
>  " >x &&

Why do you need the \<nl>? Since it is inside double quotes the shell will remove it during expansion.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Ben Walton· Oct 28, 2013, 21:10 UTC · re: Andreas Schwab · lore

Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle

On Mon, Oct 28, 2013 at 5:39 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:

Show 17 quoted lines

> Ben Walton <bdwalton@gmail.com> writes:
>
>> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
>> index 3fb4b97..0126154 100755
>> --- a/t/t4015-diff-whitespace.sh
>> +++ b/t/t4015-diff-whitespace.sh
>> @@ -145,7 +145,8 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect
>>  test_expect_success 'ignore-blank-lines: only new lines' '
>>       test_seq 5 >x &&
>>       git update-index x &&
>> -     test_seq 5 | sed "/3/i \\
>> +     test_seq 5 | sed "/3/i\\
>> +\
>>  " >x &&
>
> Why do you need the \<nl>?  Since it is inside double quotes the shell
> will remove it during expansion.

It's an escape. Without it, sed throws:

sed: -e expression #1, char 5: expected \ after `a', `c' or `i'

Thanks -Ben

-- 
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

Andreas Schwab· Oct 28, 2013, 21:59 UTC · re: Ben Walton · lore

Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle

Ben Walton <bdwalton@gmail.com> writes:

> It's an escape. Without it, sed throws:

The shell removes it before sed can see it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Junio C Hamano· Oct 30, 2013, 19:30 UTC · re: Ben Walton · lore

Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle

Ben Walton <bdwalton@gmail.com> writes:

Show 22 quoted lines

> On Mon, Oct 28, 2013 at 5:39 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Ben Walton <bdwalton@gmail.com> writes:
>>
>>> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
>>> index 3fb4b97..0126154 100755
>>> --- a/t/t4015-diff-whitespace.sh
>>> +++ b/t/t4015-diff-whitespace.sh
>>> @@ -145,7 +145,8 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect
>>>  test_expect_success 'ignore-blank-lines: only new lines' '
>>>       test_seq 5 >x &&
>>>       git update-index x &&
>>> -     test_seq 5 | sed "/3/i \\
>>> +     test_seq 5 | sed "/3/i\\
>>> +\
>>>  " >x &&
>>
>> Why do you need the \<nl>?  Since it is inside double quotes the shell
>> will remove it during expansion.
>
> It's an escape. Without it, sed throws:
>
> sed: -e expression #1, char 5: expected \ after `a', `c' or `i'

I think Andreas means the "feed blank line" part, i.e.

>> +     test_seq 5 | sed "/3/i\\
>> +\
>>  " >x &&

should be the same as

>> +     test_seq 5 | sed "/3/i\\
>>  " >x &&

because the lone \<nl> will be eaten and will not be seen by sed.

Do you see different results on Solaris between the following two?

	$ echo "/3/i\\
	\
	" | od
	$ echo "/3/i\\
        " | od

Ben Walton· Nov 3, 2013, 13:08 UTC · re: Junio C Hamano · lore

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
This addresses Andreas' comment about the extraneous \<nl>.
Sorry, I misunderstood the original comment.

 t/t4015-diff-whitespace.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Show changes to t/t4015-diff-whitespace.sh +3 −2

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 3fb4b97..604a838 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -145,7 +145,7 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect
 test_expect_success 'ignore-blank-lines: only new lines' '
 	test_seq 5 >x &&
 	git update-index x &&
-	test_seq 5 | sed "/3/i \\
+	test_seq 5 | sed "/3/i\\
 " >x &&
 	git diff --ignore-blank-lines >out &&
 	>expect &&
@@ -155,7 +155,8 @@ test_expect_success 'ignore-blank-lines: only new lines' '
 test_expect_success 'ignore-blank-lines: only new lines with space' '
 	test_seq 5 >x &&
 	git update-index x &&
-	test_seq 5 | sed "/3/i \ " >x &&
+	test_seq 5 | sed "/3/i\\
+ " >x &&
 	git diff -w --ignore-blank-lines >out &&
 	>expect &&
 	test_cmp out expect

-- 
1.8.3.2