Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes

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

 



On 16/06/17 14:49, Johannes Schindelin wrote:
> Hi Junio,
> 
> On Thu, 15 Jun 2017, Junio C Hamano wrote:
> 
>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
>> index 325ec75353..801bce25da 100755
>> --- a/t/t3420-rebase-autostash.sh
>> +++ b/t/t3420-rebase-autostash.sh
>> @@ -45,7 +45,7 @@ create_expected_success_am() {
>>  }
>>  
>>  create_expected_success_interactive() {
>> -	cr=$'\r' &&
>> +	cr=$(echo . | tr '.' '\015') &&
>>  	cat >expected <<-EOF
>>  	$(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
>>  	HEAD is now at $(git rev-parse --short feature-branch) third commit
> 
> This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015')
> would emit) is interpreted correctly as a line break on Windows, meaning
> that cr is now *empty*. Not what we want.
> 
> What I did is to replace the `cat` by `q_to_cr` (we have that lovely
> function, might just as well use it), replace `${cr}` by `Q` and skip the
> cr variable altogether.
> 
> Additionally, there is another huge problem: the "Applied autostash." is
> printed to stdout, not stderr, in line with how things were done in the
> shell version of rebase -i.
> 
> While this was just a minor bug previously, now we exercise that bug, by
> redirecting stderr to stdout and redirecting stdout to the file `actual`.
> Nothing says that stderr should be printed to that file before stdout, but
> that is exactly what the test case tries to verify.
> 
> There is only one slight problem: in my Git for Windows SDK, stdout is
> printed first.
> 
> The quick fix would be to redirect stderr and stdout independently.
> 
> However, I think it is time for that bug to be fixed: autostash messages
> should really go to stderr, just like the rest of them rebase messages.
> 
> I attached the patch, together with the two fixups to Phillip's patches,
> and a fixup for the autostash-messages-to-stderr patch that I think should
> be squashed in but I really ran out of time testing this.
> 
> Phillip, would you mind picking those changes up as you deem appropriate?

Will do, thanks for the patches




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